Quiz exercises: Fix apollon drag-and-drop quiz generation#12559
Open
FadyGergesRezk wants to merge 12 commits intodevelopfrom
Open
Quiz exercises: Fix apollon drag-and-drop quiz generation#12559FadyGergesRezk wants to merge 12 commits intodevelopfrom
Quiz exercises: Fix apollon drag-and-drop quiz generation#12559FadyGergesRezk wants to merge 12 commits intodevelopfrom
Conversation
github-actions
Bot
added
client
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds quiz-aware Apollon model utilities, uses them in diagram detail and exercise-generation, introduces SVG trimming and blanked-background PNG generation for quiz exports, updates drag-and-drop editor readiness gating, and bumps the Apollon dependency. Changes
sequenceDiagram
participant UI as Quiz UI
participant Gen as QuizExerciseGenerator
participant Util as ApollonModelUtil
participant Export as SVGExporter
participant Canvas as Canvas/Image
participant FS as File/Blob
UI->>Gen: request generate DnD exercise (model)
Gen->>Util: getQuizRelevantElementIds(model)
Util-->>Gen: list of element IDs
Gen->>Export: export full diagram SVG (svgMode: 'compat')
Export-->>Gen: fullDiagramSVG
Gen->>Canvas: createBlankedDiagramBackground(fullDiagramSVG, dropLocations)
Canvas-->>Gen: backgroundPNG/blob
loop per element
Gen->>Export: export element SVG (svgMode:'compat', margin:0)
Export->>Export: trimRenderedSVGToContent(svg)
Export-->>Gen: trimmedElementSVG + clip
Gen->>Canvas: compose item image using trimmedElementSVG and clip
Canvas-->>Gen: item image blob
end
Gen->>FS: upload/save background + items
FS-->>UI: return generated DragAndDropQuestion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/webapp/app/quiz/manage/drag-and-drop-question/drag-and-drop-question-edit.component.ts (1)
262-272:⚠️ Potential issue | 🟠 MajorDo not mark the background ready from resize/no-image paths.
adjustClickLayerWidth()is also called bywindow.onresize, not only afterImageLoadingStatus.SUCCESS. Line 271 can therefore re-enable overlays while the background is missing, failed, or still zero-sized.🐛 Proposed guard
adjustClickLayerWidth() { + const backgroundImageElement = this.backgroundImage().element.nativeElement; + if (!this.question().backgroundFilePath || backgroundImageElement.offsetWidth === 0 || backgroundImageElement.offsetHeight === 0) { + this.backgroundReady = false; + return; + } + // Make the background image visible upon successful image load. Initially it is set to hidden and not // conditionally loaded via '*ngIf' because otherwise the reference would be undefined and hence we // wouldn't be able to subscribe to the loading process updates. - this.backgroundImage().element.nativeElement.style.visibility = 'visible'; + backgroundImageElement.style.visibility = 'visible'; // Adjust the click layer to correspond to the area of the background image. - this.clickLayer().nativeElement.style.width = `${this.backgroundImage().element.nativeElement.offsetWidth}px`; - this.clickLayer().nativeElement.style.left = `${this.backgroundImage().element.nativeElement.offsetLeft}px`; + this.clickLayer().nativeElement.style.width = `${backgroundImageElement.offsetWidth}px`; + this.clickLayer().nativeElement.style.left = `${backgroundImageElement.offsetLeft}px`; this.backgroundReady = true; this.changeDetector.detectChanges(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/webapp/app/quiz/manage/drag-and-drop-question/drag-and-drop-question-edit.component.ts` around lines 262 - 272, In adjustClickLayerWidth(), avoid marking the background ready or making it visible on resize or when no image is loaded; first check that the image has actually loaded and has non-zero size (e.g., this.backgroundImage().loadingStatus === ImageLoadingStatus.SUCCESS && this.backgroundImage().element.nativeElement.offsetWidth > 0) before setting element.style.visibility = 'visible', this.backgroundReady = true, and calling this.changeDetector.detectChanges(); otherwise only update the click layer positioning/size (clickLayer().nativeElement.style.width/left) without toggling backgroundReady or visibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/webapp/app/modeling/shared/apollon-model.util.ts`:
- Around line 143-149: getQuizRelevantElementIds currently treats an empty array
from getExplicitInteractiveElementIds as “no explicit config” and falls back to
all model elements; change the logic so an explicit-but-empty interactive config
is preserved (returning an empty list) rather than defaulting to all elements.
Concretely: update getExplicitInteractiveElementIds to return a distinguishable
value (e.g., null/undefined) when no interactive configuration exists, or add a
helper like hasExplicitInteractiveConfig(model) that detects presence of
interactive.elements/interactive.relationships; then in
getQuizRelevantElementIds, check that presence and if the config exists return
the explicitInteractiveIds as-is (even if []), otherwise return
[...getModelElementIds(model)]. Ensure any callers/tests of
getExplicitInteractiveElementIds are adjusted for the new nullable/indicator
behavior.
In
`@src/main/webapp/app/quiz/manage/drag-and-drop-question/drag-and-drop-question-edit.component.html`:
- Around line 289-290: The click-layer's mousedown handler should be guarded so
clicks do nothing until the background is ready; update the template where the
element with template ref clickLayer and class "click-layer" binds
(mousedown)="backgroundMouseDown()" to only call the handler when
backgroundReady (and optionally when question().backgroundFilePath exists) —
e.g., change the binding to guard with backgroundReady before invoking
backgroundMouseDown() so drop locations cannot be created before sizing is
complete.
---
Outside diff comments:
In
`@src/main/webapp/app/quiz/manage/drag-and-drop-question/drag-and-drop-question-edit.component.ts`:
- Around line 262-272: In adjustClickLayerWidth(), avoid marking the background
ready or making it visible on resize or when no image is loaded; first check
that the image has actually loaded and has non-zero size (e.g.,
this.backgroundImage().loadingStatus === ImageLoadingStatus.SUCCESS &&
this.backgroundImage().element.nativeElement.offsetWidth > 0) before setting
element.style.visibility = 'visible', this.backgroundReady = true, and calling
this.changeDetector.detectChanges(); otherwise only update the click layer
positioning/size (clickLayer().nativeElement.style.width/left) without toggling
backgroundReady or visibility.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 23d78846-5453-4fd3-a7cf-02e4c0659b6e
📒 Files selected for processing (6)
src/main/webapp/app/modeling/shared/apollon-model.util.tssrc/main/webapp/app/quiz/manage/apollon-diagrams/detail/apollon-diagram-detail.component.tssrc/main/webapp/app/quiz/manage/apollon-diagrams/exercise-generation/quiz-exercise-generator.tssrc/main/webapp/app/quiz/manage/apollon-diagrams/exercise-generation/svg-renderer.tssrc/main/webapp/app/quiz/manage/drag-and-drop-question/drag-and-drop-question-edit.component.htmlsrc/main/webapp/app/quiz/manage/drag-and-drop-question/drag-and-drop-question-edit.component.ts
Comment on lines
+289
to
+290
| <div #clickLayer class="click-layer" (mousedown)="backgroundMouseDown()" [ngClass]="{ disabled: !question().backgroundFilePath || !backgroundReady }"> | ||
| @if (backgroundReady) { |
Contributor
There was a problem hiding this comment.
Guard the click handler while the background is not ready.
Line 289 only adds a disabled class; the handler still calls backgroundMouseDown() if pointer events are not fully blocked by CSS. Gate the call with backgroundReady so drop locations cannot be created before sizing is complete.
🐛 Proposed fix
-<div `#clickLayer` class="click-layer" (mousedown)="backgroundMouseDown()" [ngClass]="{ disabled: !question().backgroundFilePath || !backgroundReady }">
+<div `#clickLayer` class="click-layer" (mousedown)="backgroundReady && backgroundMouseDown()" [ngClass]="{ disabled: !question().backgroundFilePath || !backgroundReady }">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div #clickLayer class="click-layer" (mousedown)="backgroundMouseDown()" [ngClass]="{ disabled: !question().backgroundFilePath || !backgroundReady }"> | |
| @if (backgroundReady) { | |
| <div `#clickLayer` class="click-layer" (mousedown)="backgroundReady && backgroundMouseDown()" [ngClass]="{ disabled: !question().backgroundFilePath || !backgroundReady }"> | |
| `@if` (backgroundReady) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/webapp/app/quiz/manage/drag-and-drop-question/drag-and-drop-question-edit.component.html`
around lines 289 - 290, The click-layer's mousedown handler should be guarded so
clicks do nothing until the background is ready; update the template where the
element with template ref clickLayer and class "click-layer" binds
(mousedown)="backgroundMouseDown()" to only call the handler when
backgroundReady (and optionally when question().backgroundFilePath exists) —
e.g., change the binding to guard with backgroundReady before invoking
backgroundMouseDown() so drop locations cannot be created before sizing is
complete.
github-project-automation
Bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
End-to-End Test Results
Test Strategy: Two-phase execution
Overall: ✅ All E2E tests passed (both phases) 🔗 Workflow Run · 📊 Test Report |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/webapp/app/quiz/manage/apollon-diagrams/detail/apollon-diagram-detail.component.ts`:
- Around line 102-104: Wrap the JSON.parse + importDiagram step in a try/catch
so a corrupted diagram JSON does not throw out of the HTTP success handler:
attempt to parse diagram.jsonRepresentation and call importDiagram(...); on any
exception set model = undefined, set this.lastSavedModelJson = '' and proceed to
call this.initializeApollonEditor(model); also log or surface the parse/import
error (e.g. console.error or existing logger) so the failure is recorded instead
of breaking the detail view. Ensure you update the block that currently
references diagram.jsonRepresentation, importDiagram, this.lastSavedModelJson
and initializeApollonEditor.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7f939292-a814-496b-a109-4e7cce4ba98a
📒 Files selected for processing (1)
src/main/webapp/app/quiz/manage/apollon-diagrams/detail/apollon-diagram-detail.component.ts
Comment on lines
+102
to
104
| const model: UMLModel | undefined = diagram.jsonRepresentation ? importDiagram(JSON.parse(diagram.jsonRepresentation)) : undefined; | ||
| this.lastSavedModelJson = model ? JSON.stringify(model) : ''; | ||
| this.initializeApollonEditor(model); |
Contributor
There was a problem hiding this comment.
Handle invalid stored diagram JSON before initializing Apollon.
Line 102 can throw from JSON.parse() or importDiagram(). That exception will bypass the HTTP error callback, so a corrupted/incompatible saved diagram can break the detail view without showing the existing loading error.
Proposed guard
- const model: UMLModel | undefined = diagram.jsonRepresentation ? importDiagram(JSON.parse(diagram.jsonRepresentation)) : undefined;
- this.lastSavedModelJson = model ? JSON.stringify(model) : '';
- this.initializeApollonEditor(model);
+ let model: UMLModel | undefined;
+ try {
+ model = diagram.jsonRepresentation ? importDiagram(JSON.parse(diagram.jsonRepresentation)) : undefined;
+ } catch {
+ this.alertService.error('artemisApp.apollonDiagram.detail.error.loading');
+ return;
+ }
+
+ this.lastSavedModelJson = model ? JSON.stringify(model) : '';
+ this.initializeApollonEditor(model);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/webapp/app/quiz/manage/apollon-diagrams/detail/apollon-diagram-detail.component.ts`
around lines 102 - 104, Wrap the JSON.parse + importDiagram step in a try/catch
so a corrupted diagram JSON does not throw out of the HTTP success handler:
attempt to parse diagram.jsonRepresentation and call importDiagram(...); on any
exception set model = undefined, set this.lastSavedModelJson = '' and proceed to
call this.initializeApollonEditor(model); also log or surface the parse/import
error (e.g. console.error or existing logger) so the failure is recorded instead
of breaking the detail view. Ensure you update the block that currently
references diagram.jsonRepresentation, importDiagram, this.lastSavedModelJson
and initializeApollonEditor.
tamang29
previously approved these changes
Apr 20, 2026
tamang29
changed the title
Quiz: fix apollon drag-and-drop quiz generation
tamang29
changed the title
Quiz: fix apollon drag-and-drop quiz generationQuiz: Fix apollon drag-and-drop quiz generation
tamang29
changed the title
Quiz: Fix apollon drag-and-drop quiz generationQuiz exercises: Fix apollon drag-and-drop quiz generation
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/webapp/app/quiz/manage/apollon-diagrams/exercise-generation/quiz-exercise-generator.spec.ts (1)
151-155: Assert the background export call directly.
toHaveBeenCalledWithcan pass on any matching export call. Since per-element exports may also usesvgMode: 'compat', assert the first/background call and that it is not aninclude/excludeexport.🧪 Proposed assertion tightening
await generateDragAndDropQuizExercise(course, 'Background Test', v3Model); // The new renderer exports the full model and blanks the interactive regions afterward. - expect(mockExportModelAsSvg).toHaveBeenCalledWith(v3Model, expect.objectContaining({ keepOriginalSize: true, svgMode: 'compat' })); + const backgroundExportOptions = mockExportModelAsSvg.mock.calls[0][1]; + expect(backgroundExportOptions).toEqual(expect.objectContaining({ keepOriginalSize: true, svgMode: 'compat' })); + expect(backgroundExportOptions).not.toHaveProperty('include'); + expect(backgroundExportOptions).not.toHaveProperty('exclude');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/webapp/app/quiz/manage/apollon-diagrams/exercise-generation/quiz-exercise-generator.spec.ts` around lines 151 - 155, The test currently uses toHaveBeenCalledWith which can match any export call; instead assert the first/background export directly by inspecting mockExportModelAsSvg.mock.calls[0] after calling generateDragAndDropQuizExercise(course, 'Background Test', v3Model), and verify the second argument (options) contains keepOriginalSize: true and svgMode: 'compat' and does NOT include per-element include/exclude flags (e.g., no includeInteractiveElements or excludeInteractiveElements keys); update the assertion in quiz-exercise-generator.spec.ts to target mockExportModelAsSvg.mock.calls[0][1] so the background export is unambiguous.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/webapp/app/quiz/manage/apollon-diagrams/exercise-generation/quiz-exercise-generator.spec.ts`:
- Around line 109-112: The module-scoped vi.fn() mockExportModelAsSvg retains
call history across tests (Vitest v4's vi.restoreAllMocks() doesn't clear
calls), causing order-dependent assertions; fix by clearing or resetting that
mock at the start of each test suite or in beforeEach (call
mockExportModelAsSvg.mockClear() or mockExportModelAsSvg.mockReset(), or
vi.clearAllMocks()) and re-spy ApollonEditor.exportModelAsSvg as needed so tests
reference only current-test calls; apply the same reset to the other occurrences
around lines 246-250 where mockExportModelAsSvg is used.
---
Nitpick comments:
In
`@src/main/webapp/app/quiz/manage/apollon-diagrams/exercise-generation/quiz-exercise-generator.spec.ts`:
- Around line 151-155: The test currently uses toHaveBeenCalledWith which can
match any export call; instead assert the first/background export directly by
inspecting mockExportModelAsSvg.mock.calls[0] after calling
generateDragAndDropQuizExercise(course, 'Background Test', v3Model), and verify
the second argument (options) contains keepOriginalSize: true and svgMode:
'compat' and does NOT include per-element include/exclude flags (e.g., no
includeInteractiveElements or excludeInteractiveElements keys); update the
assertion in quiz-exercise-generator.spec.ts to target
mockExportModelAsSvg.mock.calls[0][1] so the background export is unambiguous.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 09070955-ddb3-44f8-9b74-a6244015f49d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
src/main/webapp/app/modeling/shared/apollon-model.util.tssrc/main/webapp/app/quiz/manage/apollon-diagrams/detail/apollon-diagram-detail.component.spec.tssrc/main/webapp/app/quiz/manage/apollon-diagrams/exercise-generation/quiz-exercise-generator.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/webapp/app/modeling/shared/apollon-model.util.ts
Comment on lines
109
to
+112
| // Mock static method with proper cleanup | ||
| vi.spyOn(ApollonEditor, 'exportModelAsSvg').mockImplementation(mockExportModelAsSvg); | ||
| vi.spyOn(SVGRendererAPI, 'convertRenderedSVGToPNG').mockResolvedValue(new Blob(['PNG'], { type: 'image/png' })); | ||
| cleanupCanvasAndImageMocks = setupCanvasAndImageMocks().cleanup; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find module-scoped vi.fn mocks whose call history may be inspected across tests.
rg -n -C3 --type=ts 'const\s+\w+\s*=\s*vi\.fn\(|\.mock\.calls|restoreAllMocks\(\)'Repository: ls1intum/Artemis
Length of output: 50373
🏁 Script executed:
# Get file size first
wc -l src/main/webapp/app/quiz/manage/apollon-diagrams/exercise-generation/quiz-exercise-generator.spec.tsRepository: ls1intum/Artemis
Length of output: 164
🏁 Script executed:
# Read the file to find mockExportModelAsSvg declaration and the problematic lines
cat -n src/main/webapp/app/quiz/manage/apollon-diagrams/exercise-generation/quiz-exercise-generator.spec.ts | head -120 | tail -40Repository: ls1intum/Artemis
Length of output: 1854
🏁 Script executed:
# Check around lines 109-112
sed -n '100,120p' src/main/webapp/app/quiz/manage/apollon-diagrams/exercise-generation/quiz-exercise-generator.spec.tsRepository: ls1intum/Artemis
Length of output: 867
🏁 Script executed:
# Check around lines 154-155
sed -n '145,160p' src/main/webapp/app/quiz/manage/apollon-diagrams/exercise-generation/quiz-exercise-generator.spec.tsRepository: ls1intum/Artemis
Length of output: 894
🏁 Script executed:
# Check around lines 246-250
sed -n '240,255p' src/main/webapp/app/quiz/manage/apollon-diagrams/exercise-generation/quiz-exercise-generator.spec.tsRepository: ls1intum/Artemis
Length of output: 836
🏁 Script executed:
# Find mockExportModelAsSvg declaration
rg -n 'mockExportModelAsSvg' src/main/webapp/app/quiz/manage/apollon-diagrams/exercise-generation/quiz-exercise-generator.spec.ts | head -20Repository: ls1intum/Artemis
Length of output: 524
🏁 Script executed:
# Let me check the actual test structure to understand the call dependency better
sed -n '240,260p' src/main/webapp/app/quiz/manage/apollon-diagrams/exercise-generation/quiz-exercise-generator.spec.tsRepository: ls1intum/Artemis
Length of output: 1061
🏁 Script executed:
# Check how many tests exist that call mockExportModelAsSvg
grep -n "it('" src/main/webapp/app/quiz/manage/apollon-diagrams/exercise-generation/quiz-exercise-generator.spec.ts | wc -lRepository: ls1intum/Artemis
Length of output: 62
🏁 Script executed:
# List all tests to understand the structure
grep -n "it('" src/main/webapp/app/quiz/manage/apollon-diagrams/exercise-generation/quiz-exercise-generator.spec.tsRepository: ls1intum/Artemis
Length of output: 1937
🌐 Web query:
Vitest 4 vi.restoreAllMocks() behavior clear mock history calls
💡 Result:
In Vitest 4, vi.restoreAllMocks restores original implementations only for spies created with vi.spyOn. It does not clear mock call history (mock.calls, mock.results, etc.). To clear call history without affecting implementations, use vi.clearAllMocks. To clear history and reset implementations to empty functions, use vi.resetAllMocks. This is a change from previous versions where restoreAllMocks also reset state; now it only restores vi.spyOn spies, and automocks are unaffected. Module mocks from vi.mock are hoisted and not affected by any of these; use vi.doMock/doUnmock for dynamic control.
Citations:
- 1: https://v4.vitest.dev/api/vi
- 2: https://vitest.dev/guide/migration.html
- 3: https://vitest.dev/api/vi
- 4: https://v4.vitest.dev/api/mock
Reset mockExportModelAsSvg before each test to prevent order-dependent mock assertions.
mockExportModelAsSvg is a module-scoped vi.fn(), and vi.restoreAllMocks() in Vitest 4 does not clear mock call history. Tests that read mockExportModelAsSvg.mock.calls[0] can reference calls from earlier tests, making assertions order-dependent and fragile.
🧪 Proposed fix
+ const defaultExportModelAsSvgResult = {
+ svg: '<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100"></svg>',
+ clip: { x: 0, y: 0, width: 100, height: 100 },
+ };
+
beforeEach(async () => {
await TestBed.configureTestingModule({
providers: [
provideHttpClient(),
provideHttpClientTesting(),
@@
}).compileComponents();
+ mockExportModelAsSvg.mockReset();
+ mockExportModelAsSvg.mockResolvedValue(defaultExportModelAsSvgResult);
+
// Mock static method with proper cleanup
vi.spyOn(ApollonEditor, 'exportModelAsSvg').mockImplementation(mockExportModelAsSvg);
vi.spyOn(SVGRendererAPI, 'convertRenderedSVGToPNG').mockResolvedValue(new Blob(['PNG'], { type: 'image/png' }));
cleanupCanvasAndImageMocks = setupCanvasAndImageMocks().cleanup;Also applies to: 246-250
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/webapp/app/quiz/manage/apollon-diagrams/exercise-generation/quiz-exercise-generator.spec.ts`
around lines 109 - 112, The module-scoped vi.fn() mockExportModelAsSvg retains
call history across tests (Vitest v4's vi.restoreAllMocks() doesn't clear
calls), causing order-dependent assertions; fix by clearing or resetting that
mock at the start of each test suite or in beforeEach (call
mockExportModelAsSvg.mockClear() or mockExportModelAsSvg.mockReset(), or
vi.clearAllMocks()) and re-spy ApollonEditor.exportModelAsSvg as needed so tests
reference only current-test calls; apply the same reset to the other occurrences
around lines 246-250 where mockExportModelAsSvg is used.
dogusaytok
reviewed
Apr 22, 2026
| this.apollonDiagram.set(diagram); | ||
|
|
||
| const model: UMLModel = diagram.jsonRepresentation ? JSON.parse(diagram.jsonRepresentation) : undefined; | ||
| const model: UMLModel | undefined = diagram.jsonRepresentation ? importDiagram(JSON.parse(diagram.jsonRepresentation)) : undefined; |
There was a problem hiding this comment.
Using importDiagram() here can drop interactive class attributes and methods from legacy v3 diagrams. Because the import folds them into node.data, the new generator flow (which only checks top-level nodes and edges) can miss them, and may cause users to lose existing quiz gaps just by reopening an older diagram, right?
For that, I think we can either preserve the raw v3 model for quiz selection and generation, or update the helper methods to traverse and display those nested attribute IDs. What do you think?
dogusaytok
reviewed
Apr 22, 2026
Comment on lines
+204
to
+211
| for (const dropLocation of dropLocations) { | ||
| context.fillRect( | ||
| (dropLocation.posX! / MAX_SIZE_UNIT) * canvas.width, | ||
| (dropLocation.posY! / MAX_SIZE_UNIT) * canvas.height, | ||
| (dropLocation.width! / MAX_SIZE_UNIT) * canvas.width, | ||
| (dropLocation.height! / MAX_SIZE_UNIT) * canvas.height, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Blanking elements using fillRect() and bounding boxes may break relationship-based questions. An edge's bounding box can cover a large area, right? This may accidentally hide unrelated nodes or labels below it.
We can either use the exact SVG path to hide elements or stick with the previous method of excluding elements individually. What do you think?
dogusaytok
reviewed
Apr 22, 2026
| dueDate: quizExercise.dueDate, | ||
| includedInOverallScore: quizExercise.includedInOverallScore, | ||
| quizQuestions: quizExercise.quizQuestions, | ||
| quizQuestions: quizExercise.quizQuestions ? convertQuizQuestionsToDTOs(quizExercise.quizQuestions) : undefined, |
There was a problem hiding this comment.
Using the creation converter again removes question ids and temporary DnD ids that were already there. This may cause the server to see all edited questions as new. This change resets their statistics and may cause problems in DnD mappings because saved items will drop to 0.
We might use a specific update DTO that keeps these ids, so that we can match the backend's update contract I think.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR updates the Apollon quiz import flow so Artemis explicitly opts into Apollon quiz mode, correctly derives quiz-relevant elements from the editor model, and generates drag-and-drop questions with properly aligned backgrounds, drop locations, and drag item images.
Checklist
General
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Motivation and Context
Generating drag-and-drop quiz questions from Apollon diagrams was unreliable in the Artemis quiz flow.
The main issues were:
Description
This PR updates the Artemis integration with Apollon for drag-and-drop quiz generation:
enableQuizMode: true.importDiagram(...)when loading stored Apollon diagrams in the quiz dialog.model.interactive, with fallback to model elements when needed.svgMode: 'compat'for quiz-related SVG exports to preserve correct colors in generated images.Steps for Testing
Prerequisites:
ApollonQuizDragAndDropfeature toggle if required in the test environmentGenerate Quiz Exercisebecomes enabled.Exam Mode Testing
Prerequisites:
Review Progress
Performance Review
Code Review
Manual Tests
Exam Mode Test
Performance Tests
Screenshots
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores