`Development`: Add client support for programming exercise feedback suggestions #7099

pal03377 · 2023-08-26T01:02:20Z

Note
The hardcoded example feedback suggestions will need to be removed before merging.

Quick links:

Feature proposal: Programming exercises: Add Athena Feedback Suggestions #7094
Client (this PR): Development: Add client support for programming exercise feedback suggestions #7099
Server: Programming exercises: Add server support for feedback suggestions #7136
Client + Server merged: Programming exercises: Add support for feedback suggestions (both other PRs combined) #7138

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

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

See #7094. We want to implement programming feedback suggestions on the client. Also, this PR refactors some parts of the code editor assessment area, making it more robust.

Description

This PR implements the client side of Programming exercises: Add Athena Feedback Suggestions #7094 (feature proposal).
It also fixes Programming exercises: Cancelling feedback can go back too many steps #7102
Because of refactoring, it might fix Programming exercises: Inline feedback sticks to top of online editor #7033 as a byproduct (inline feedback sometimes "escapes" editor).
It resolves Programming exercises, Assessment: Unnecessary empty space below tutor comments #6367: I slightly adjusted the spacing below the feedback cards.

Steps for Testing

Prerequisites:

1 TA
1 Programming Exercise with manual assessment enabled and with submissions to assess. The package name has to be de.athena and include BubbleSort.java because otherwise, the hardcoded example feedback suggestion will point nowhere!

Log in to Artemis
Go into the exercise dashboard and start the assessment
Play around with the feedback in the editor:

Create lots of inline feedback
Create general feedback
Switch between files rapidly
Save, Cancel, Delete feedback
Rename files
Collapse folders

Test referenced feedback suggestions: You will see hardcoded referenced feedback suggestions in BubbleSort.java and MergeSort.java. Accept or discard them. You can get it back by not saving the whole assessment and refreshing the page. It is expected that discarded feedback re-appears this way. Try editing it as well. Check that the feedback suggestion badge always appropriately represents the state of the suggestion. Also, switch around in files again.
Test unreferenced feedback suggestions the same way. There is one unreferenced feedback suggestion at the bottom.
Save the assessment. Refresh and inspect the feedback on the page. Edit it again.
Look at the feedback from the perspective of a student.
Check that the feedback suggestions don't influence the overall score until they are accepted.
Collapse the src/de/athena folder. The feedback suggestions badge should sum up the badges of all files and show up next to the collapsed folder.

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

Client

Class/File	Line Coverage	Confirmation (assert/expect)
assessment-detail.component.ts	not found (deleted)
athena.service.ts		Currently incorrect because of testing data, should be close to 100%
assessment-correction-round-badge.component.ts	not found (renamed)
unreferenced-feedback-detail.component.ts	100%	✅
feedback.model.ts	96.69%	✅
code-editor-tutor-assessment-container.component.ts	86.19%	✅ up from ~85%
code-editor-tutor-assessment-inline-feedback-suggestion.component.ts	100%
code-editor-tutor-assessment-inline-feedback.component.ts	98.11%	✅
code-editor-ace.component.ts	84.27%	✅ up from ~81%
code-editor-container.component.ts	86.36%	✅ up from ~85%
code-editor-file-browser-badge.component.ts	100%
code-editor-file-browser-file.component.ts	100%	✅
code-editor-file-browser.component.ts	86.03%	✅ up from ~85%
code-editor.model.ts	100%	✅
feedback-suggestion-badge.component.ts	100%
unreferenced-feedback.component.ts	96.55%	✅
textblock-feedback-editor.component.ts	92.98%	✅

Screenshots

Student view: No feedback suggestion badges:

…ggestions

pal03377

I added a few notes for code reviewers to make it easier to understand changes.

pal03377 · 2023-08-27T23:26:31Z

src/main/webapp/app/assessment/assessment-detail/assessment-detail.component.html

@@ -1,71 +0,0 @@
-<div (drop)="updateAssessmentOnDrop($event)" (dragover)="$event.preventDefault()" class="card mb-3" id="assessment-detail">


Note for code reviewers: I renamed this component to unreferenced-feedback-detail, as that's what it really is.

pal03377 · 2023-08-27T23:27:52Z

src/main/webapp/app/assessment/assessment-shared.module.ts

@@ -25,21 +26,22 @@ const ENTITY_STATES = [...assessmentLocksRoute];
        RouterModule.forChild(ENTITY_STATES),
        ArtemisMarkdownModule,
        ArtemisGradingInstructionLinkIconModule,
+        ArtemisFeedbackModule,


Note for code reviewers: This is needed to get the new feedback-suggestion-badge component.

pal03377 · 2023-08-27T23:28:49Z

src/main/webapp/app/assessment/athena.service.ts

+        // For debugging: Return basic feedback suggestions for BubbleSort.java
+        const referencedFeedbackSuggestion = new Feedback();
+        referencedFeedbackSuggestion.id = 1;
+        referencedFeedbackSuggestion.credits = 1;
+        referencedFeedbackSuggestion.text = 'FeedbackSuggestion:';
+        referencedFeedbackSuggestion.detailText = 'This is a referenced feedback suggestion - test test';
+        referencedFeedbackSuggestion.gradingInstruction = undefined;
+        referencedFeedbackSuggestion.reference = 'file:src/de/athena/BubbleSort.java_line:14';
+        referencedFeedbackSuggestion.type = FeedbackType.AUTOMATIC;
+        const unreferencedFeedbackSuggestion = new Feedback();
+        unreferencedFeedbackSuggestion.id = 2;
+        unreferencedFeedbackSuggestion.credits = -1;
+        unreferencedFeedbackSuggestion.text = 'FeedbackSuggestion:';
+        unreferencedFeedbackSuggestion.detailText = 'This is an unreferenced feedback suggestion';
+        unreferencedFeedbackSuggestion.gradingInstruction = undefined;
+        unreferencedFeedbackSuggestion.reference = undefined;
+        unreferencedFeedbackSuggestion.type = FeedbackType.AUTOMATIC;
+        return of([referencedFeedbackSuggestion, unreferencedFeedbackSuggestion]);


This must and will be removed before the PR is merged. It's nice for testing, though.

pal03377 · 2023-08-27T23:32:05Z

...bapp/app/assessment/unreferenced-feedback-detail/unreferenced-feedback-detail.component.html

+<div (drop)="updateFeedbackOnDrop($event)" (dragover)="$event.preventDefault()" class="card mb-3" [class.is-suggestion]="isSuggestion" id="assessment-detail">
+    <div class="card-header">
+        <jhi-feedback-suggestion-badge *ngIf="isSuggestion || Feedback.isFeedbackSuggestion(feedback)" [feedback]="feedback"></jhi-feedback-suggestion-badge>
+        <jhi-grading-instruction-link-icon *ngIf="feedback.gradingInstruction" [feedback]="feedback"></jhi-grading-instruction-link-icon>
+        <fa-icon [icon]="faTrashAlt" class="float-end" (click)="delete()" *ngIf="!readOnly"></fa-icon>
+        <!-- Accept/Discard for feedback suggestions -->
+        <div class="row float-end suggestion-action-buttons" *ngIf="isSuggestion">
+            <button class="btn btn-success m-1" (click)="onAcceptSuggestion.emit(feedback)">
+                <fa-icon [icon]="faCheck"></fa-icon>
+                <span jhiTranslate="artemisApp.assessment.detail.accept">Accept</span>
+            </button>
+            <button class="btn btn-danger m-1" (click)="onDiscardSuggestion.emit(feedback)">
+                <fa-icon [icon]="faTrash"></fa-icon>
+                <span jhiTranslate="artemisApp.assessment.detail.discard">Discard</span>
+            </button>
+        </div>
+    </div>


Note for code reviewers: The card-header and the is-suggestion class are the only new parts apart from the rename of the component from assessment-detail to unreferenced-feedback-detail

pal03377 · 2023-08-27T23:35:33Z

src/main/webapp/app/entities/feedback.model.ts

-        return !feedback.text.includes(STATIC_CODE_ANALYSIS_FEEDBACK_IDENTIFIER, 0) && !feedback.text.includes(SUBMISSION_POLICY_FEEDBACK_IDENTIFIER, 0);
+        return !feedback.text.startsWith(STATIC_CODE_ANALYSIS_FEEDBACK_IDENTIFIER) && !feedback.text.startsWith(SUBMISSION_POLICY_FEEDBACK_IDENTIFIER);


Note for code reviewers: This is just a nicer way to write it

pal03377 · 2023-08-28T00:16:45Z

...pp/app/exercises/programming/shared/code-editor/container/code-editor-container.component.ts

@@ -202,10 +236,6 @@ export class CodeEditorContainerComponent implements ComponentCanDeactivate {
        this.onFileChanged.emit();
    }

-    updateFeedback(feedbacks: Feedback[]) {


Note for code reviewers: We now call onUpdateFeedback.emit directly

pal03377 · 2023-08-28T00:17:59Z

...es/programming/shared/code-editor/file-browser/code-editor-file-browser-badge.component.html

@@ -0,0 +1,4 @@
+<span class="badge" [class.on-colored-background]="onColoredBackground" [ngbTooltip]="tooltip">


Note for code reviewers: That's the badge on the right:

pal03377 · 2023-08-28T00:20:59Z

...ses/programming/shared/code-editor/file-browser/code-editor-file-browser-file.component.html

    <!-- File level actions -->
-    <span class="file-icons" [ngClass]="item.checked ? 'text-white' : 'primary'">
-        <button [disabled]="disableActions" (click)="setRenamingNode($event)" class="btn btn-small">
+    <span class="file-icons" [ngClass]="item.checked ? 'text-white' : 'primary'" *ngIf="!disableActions">


Note for code reviewers: We now hide the file actions completely if they are not available: A tutor doesn't need to see a rename button they can't press anyway.

pal03377 · 2023-08-28T00:27:43Z

src/main/webapp/app/exercises/shared/unreferenced-feedback/unreferenced-feedback.component.html

@@ -3,25 +3,31 @@
        <button id="add-unreferenced-feedback" class="btn btn-success mt-4" (click)="addUnreferencedFeedback()" [disabled]="readOnly">
            {{ 'artemisApp.fileUploadAssessment.addFeedback' | artemisTranslate }}
        </button>
-        <ng-container *ngIf="!busy">


Note for code reviewers: The busy property was never set to true, so I removed this.

Only the part with <div *ngFor="let suggestion of feedbackSuggestions" class="col-12 col-lg-6 col-xl-6"> (line 22+) is new.

pal03377 · 2023-08-28T00:31:43Z

...p/app/exercises/text/assess/textblock-feedback-editor/textblock-feedback-editor.component.ts

    faExclamation = faExclamation;
-    faBalanceScaleRight = faBalanceScaleRight;


Note for code reviewers: This was just unused

bassner

Hey, did a quick code review. Retesting should not be necessary for these changes

...bapp/app/assessment/unreferenced-feedback-detail/unreferenced-feedback-detail.component.html

.../webapp/app/exercises/programming/assess/code-editor-tutor-assessment-container.component.ts

...es/programming/assess/code-editor-tutor-assessment-inline-feedback-suggestion.component.html

…ggestions

pal03377 added 3 commits August 25, 2023 23:29

Basic programming feedback suggestion card layout

9e6a7b9

Better Suggestion badge for feedback suggestion cards

7b37b58

Add Accept and Discard buttons

9ea12ef

pal03377 added feature component:Programming labels Aug 26, 2023

pal03377 self-assigned this Aug 26, 2023

github-actions bot added the client Pull requests that update TypeScript code. (Added Automatically!) label Aug 26, 2023

pal03377 added 2 commits August 26, 2023 15:47

Add file browser badges for suggestions

cbd820d

Fixing issues with saving inline feedback

081b104

github-actions bot added the tests label Aug 26, 2023

pal03377 mentioned this pull request Aug 27, 2023

Programming exercises: Cancelling feedback can go back too many steps #7102

Closed

Make the programming feedback assessment editor more solid

9f2e4c6

pal03377 mentioned this pull request Aug 27, 2023

Programming exercises, Assessment: Unnecessary empty space below tutor comments #6367

Closed

pal03377 and others added 8 commits August 27, 2023 06:00

Merge branch 'develop' into feature/programming-exercises/feedback-su…

86c2fa9

…ggestions

Add suggestion badge to accepted and modified feedback suggestions

afd07b2

Use suggestion badge component for text feedback

deb5678

Update file badges less often

5c64eab

Add file badge tooltips

8b09917

Show unreferenced feedback suggestions

d0ef16c

Change ID for unreferenced feedback

2c2899e

Improvements from self-code-review ✨

3a49e76

github-actions bot added the cypress Pull requests that update cypress tests. (Added Automatically!) label Aug 28, 2023

pal03377 changed the title ~~Programming Exercises: Add support for Athena feedback suggestions~~ Programming Exercises: Add client-side support for feedback suggestions Aug 28, 2023

pal03377 commented Aug 28, 2023

View reviewed changes

pal03377 added 4 commits August 28, 2023 03:50

Write client tests: part 1

f81f3d3

Write client tests: part 2

944b153

Text fix by reordering

c5bb462

Fix Codacy issue

c4d004b

pal03377 added the deploy:artemis-test4 label Aug 28, 2023

pal03377 dismissed stale reviews from rstief, tobias-lippert, rriyaldhi, MaximilianAnzinger, b-fein, MarkusPaulsen, and terlan98 via b4932cc September 28, 2023 16:18

pal03377 added the ready to merge label Sep 28, 2023

b-fein added this to the 6.5.4 milestone Sep 28, 2023

bassner requested changes Sep 28, 2023

View reviewed changes

pal03377 added 2 commits September 28, 2023 22:16

Implement Patrick's suggestions

8fb21bf

Fix cypress test

8d97684

pal03377 added the deploy:artemis-test4 label Sep 28, 2023

github-actions bot removed the deploy:artemis-test4 label Sep 28, 2023

This comment was marked as outdated.

Sign in to view

github-actions bot added the deployment-error Added by deployment workflows if an error occured label Sep 28, 2023

pal03377 requested a review from bassner September 28, 2023 21:34

pal03377 removed the deployment-error Added by deployment workflows if an error occured label Sep 28, 2023

bassner and others added 6 commits September 29, 2023 10:26

Merge branch 'develop' into feature/programming-exercises/feedback-su…

ff380d5

…ggestions

Merge branch 'develop' into feature/programming-exercises/feedback-su…

d476108

…ggestions

Merge branch 'develop' into feature/programming-exercises/feedback-su…

34dcaaa

…ggestions

Merge branch 'develop' into feature/programming-exercises/feedback-su…

27654e1

…ggestions

Merge branch 'develop' into feature/programming-exercises/feedback-su…

0e5f8bb

…ggestions

Merge branch 'develop' into feature/programming-exercises/feedback-su…

09d6b39

…ggestions

krusche modified the milestones: 6.5.4, 6.6.0 Oct 3, 2023

Merge branch 'develop' into feature/programming-exercises/feedback-su…

b45e160

…ggestions

krusche merged commit 14cf78c into develop Oct 4, 2023
15 of 18 checks passed

krusche deleted the feature/programming-exercises/feedback-suggestions branch October 4, 2023 16:08

b-fein mentioned this pull request Oct 16, 2023

Programming exercises: Inline feedback sticks to top of online editor #7033

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`Development`: Add client support for programming exercise feedback suggestions #7099

`Development`: Add client support for programming exercise feedback suggestions #7099

pal03377 commented Aug 26, 2023 •

edited by MarkusPaulsen

Loading

pal03377 left a comment

pal03377 Aug 27, 2023

pal03377 Aug 27, 2023

pal03377 Aug 27, 2023

pal03377 Aug 27, 2023

pal03377 Aug 27, 2023

pal03377 Aug 28, 2023

pal03377 Aug 28, 2023

pal03377 Aug 28, 2023

pal03377 Aug 28, 2023

pal03377 Aug 28, 2023

pal03377 Aug 28, 2023

bassner left a comment

This comment was marked as outdated.

		@@ -1,71 +0,0 @@
		<div (drop)="updateAssessmentOnDrop($event)" (dragover)="$event.preventDefault()" class="card mb-3" id="assessment-detail">

		return !feedback.text.includes(STATIC_CODE_ANALYSIS_FEEDBACK_IDENTIFIER, 0) && !feedback.text.includes(SUBMISSION_POLICY_FEEDBACK_IDENTIFIER, 0);
		return !feedback.text.startsWith(STATIC_CODE_ANALYSIS_FEEDBACK_IDENTIFIER) && !feedback.text.startsWith(SUBMISSION_POLICY_FEEDBACK_IDENTIFIER);

		@@ -0,0 +1,4 @@
		<span class="badge" [class.on-colored-background]="onColoredBackground" [ngbTooltip]="tooltip">

		faExclamation = faExclamation;
		faBalanceScaleRight = faBalanceScaleRight;

Development: Add client support for programming exercise feedback suggestions #7099

Development: Add client support for programming exercise feedback suggestions #7099

Conversation

pal03377 commented Aug 26, 2023 • edited by MarkusPaulsen Loading

Checklist

General

Client

Motivation and Context

Description

Steps for Testing

Review Progress

Performance Review

Code Review

Manual Tests

Test Coverage

Client

Screenshots

pal03377 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bassner left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

`Development`: Add client support for programming exercise feedback suggestions #7099

`Development`: Add client support for programming exercise feedback suggestions #7099

pal03377 commented Aug 26, 2023 •

edited by MarkusPaulsen

Loading