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

Assessment: Fix an issue where deleting one unsaved inline feedback deletes all unsaved feedbacks #7716

Merged
merged 12 commits into from
Dec 9, 2023

Conversation

chrisknedl
Copy link
Contributor

@chrisknedl chrisknedl commented Dec 4, 2023

Checklist

General

Client

  • I followed the 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 documented the TypeScript code using JSDoc style.

Motivation and Context

Closes #7709

Description

To correctly identify the feedback that should be deleted before it has been saved for the first time, the line, file and content of the feedback are now taken into consideration in addition to the id.

Steps for Testing

Prerequisites:

  • 1 Tutor/Instructor
  • 1 Programming Exercise with manual assessment
  1. As tutor/instructor assess a programming exercise.
  2. Add a few inline comments in the code editor.
  3. Delete one of those comments.
  4. The other comments should still be there.

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

@chrisknedl chrisknedl requested a review from a team as a code owner December 4, 2023 09:51
@chrisknedl chrisknedl added priority:high client Pull requests that update TypeScript code. (Added Automatically!) labels Dec 4, 2023
@chrisknedl chrisknedl changed the title Manual assessment: Fix an issue where deleting an unsaved comment deletes all unsaved comments Manual assessment: Fix an issue where deleting an unsaved inline feedback deletes all unsaved inline feedbacks Dec 4, 2023
@b-fein b-fein changed the title Manual assessment: Fix an issue where deleting an unsaved inline feedback deletes all unsaved inline feedbacks Assessment: Fix an issue where deleting an unsaved inline feedback deletes all unsaved inline feedbacks Dec 4, 2023
@chrisknedl chrisknedl changed the title Assessment: Fix an issue where deleting an unsaved inline feedback deletes all unsaved inline feedbacks Programming exercises, Manual assessment: Fix an issue where deleting a single unsaved comment deletes all unsaved comments Dec 4, 2023
@b-fein b-fein changed the title Programming exercises, Manual assessment: Fix an issue where deleting a single unsaved comment deletes all unsaved comments Assessment: Fix an issue where deleting an unsaved inline feedback deletes all unsaved feedbacks Dec 4, 2023
@bassner
Copy link
Member

bassner commented Dec 4, 2023

Hijacking this PR for a short test, will revert afterwards :D

@bassner bassner force-pushed the fix-delete-feedback branch from 94d8af6 to c0b801e Compare December 4, 2023 19:16
@ls1intum ls1intum deleted a comment from github-actions bot Dec 4, 2023
@ls1intum ls1intum deleted a comment from github-actions bot Dec 4, 2023
@bassner bassner force-pushed the fix-delete-feedback branch from c0b801e to 7530b35 Compare December 4, 2023 19:23
LenaKahle
LenaKahle previously approved these changes Dec 4, 2023
Copy link

@LenaKahle LenaKahle left a comment

Choose a reason for hiding this comment

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

Approve, tested on legacy ts11.

sarpsahinalp
sarpsahinalp previously approved these changes Dec 6, 2023
Copy link
Contributor

@sarpsahinalp sarpsahinalp 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 ts5. Worked as expected.

georgyia
georgyia previously approved these changes Dec 6, 2023
Copy link

@georgyia georgyia left a comment

Choose a reason for hiding this comment

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

tested manually on ts2, works as expected

@b-fein b-fein requested a review from JohannesStoehr December 8, 2023 13:19
@krusche krusche modified the milestones: 6.7.2, 6.7.3 Dec 8, 2023
vinceclifford
vinceclifford previously approved these changes Dec 8, 2023
Copy link

@vinceclifford vinceclifford 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 TS2, works as expected

AntonGeTUM
AntonGeTUM previously approved these changes Dec 8, 2023
Copy link

@AntonGeTUM AntonGeTUM 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 ts11, works as described

@krusche krusche changed the title Assessment: Fix an issue where deleting an unsaved inline feedback deletes all unsaved feedbacks Assessment: Fix an issue where deleting one unsaved inline feedback deletes all unsaved feedbacks Dec 9, 2023
@krusche krusche added the maintainer-approved The feature maintainer has approved the PR label Dec 9, 2023
@krusche
Copy link
Member

krusche commented Dec 9, 2023

just waiting for the checks to pass, then I will merge this PR

@krusche krusche merged commit 0ea0765 into develop Dec 9, 2023
32 of 36 checks passed
@krusche krusche deleted the fix-delete-feedback branch December 9, 2023 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix client Pull requests that update TypeScript code. (Added Automatically!) component:Programming maintainer-approved The feature maintainer has approved the PR priority:high ready to merge small tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Programming exercises, Manual assessment: Deleting a single unsaved comment deletes all unsaved comments