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

Programming exercises: Improve error messages for connection issues in online code editor #6888

Merged
merged 5 commits into from
Jul 22, 2023

Conversation

JohannesStoehr
Copy link
Contributor

@JohannesStoehr JohannesStoehr commented Jul 13, 2023

Checklist

General

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

Students should not be confused when the connection issues to the Artemis server cause the online editor to malfunction. So the error messages should be improved.

Description

Whenever an action is not possible in the code editor due to a connection issue an error is displayed at the top right telling the student about this problem.
Additionally when trying to load a file without an internet connection a blank file is shown instead of the last loaded file as before this PR. This blank file is read only.

Steps for Testing

Prerequisites:

  • 1 Instructor
  • 1 Student
  • 1 Programming Exercise with online editor enabled
  1. Log in to Artemis
  2. Participate using the online editor
  3. Loose internet connection by turning of wifi or stopping the server when testing locally
  4. Interact with the code editor and check that all actions (submitting, loading an unloaded file, ...) cause an error message at the top right
  5. Loading and editing existing files that were loaded before the connection loss should still be possible
  6. Reconnect to the server
  7. Check that any changes during the connection loss can now be successfully submitted

Exam Mode Testing

Prerequisites:

  • 1 Instructor
  • 1 Student
  • 1 Exam with a Programming Exercise
  1. Same as for normal exercises but within an exam

Review Progress

Performance Review

  • I confirm that the client changes (in particular related to REST calls and UI responsiveness) are implemented with a very good performance

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Exam Mode Test

  • Test 1
  • Test 2

Test Coverage

Client

Class/File Line Coverage Confirmation (assert/expect)
code-editor-ace.component.ts 80.83%
code-editor-container.component.ts 85.29%

Screenshots

Bildschirmfoto 2023-07-13 um 11 56 36

@JohannesStoehr JohannesStoehr added tests ready for review client Pull requests that update TypeScript code. (Added Automatically!) component:Programming component:Exam Mode labels Jul 13, 2023
@JohannesStoehr JohannesStoehr added this to the 6.3.4 milestone Jul 13, 2023
@JohannesStoehr JohannesStoehr requested a review from a team as a code owner July 13, 2023 10:03
@JohannesStoehr JohannesStoehr self-assigned this Jul 13, 2023
rriyaldhi
rriyaldhi previously approved these changes Jul 13, 2023
Copy link
Contributor

@rriyaldhi rriyaldhi left a comment

Choose a reason for hiding this comment

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

Code looks good.

Copy link

@YeetTheFirst21 YeetTheFirst21 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, all seems to be working as expected

checked both normal coding exercises and exam mode.

Copy link
Contributor

@MaximilianAnzinger MaximilianAnzinger left a comment

Choose a reason for hiding this comment

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

Code LGTM

@RY997 RY997 temporarily deployed to artemis-test4.artemis.cit.tum.de July 20, 2023 14:16 — with GitHub Actions Inactive
Copy link
Contributor

@RY997 RY997 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 ts4, worked well.

@krusche krusche removed this from the 6.3.5 milestone Jul 21, 2023
@krusche krusche added this to the 6.3.6 milestone Jul 21, 2023
@DominikRemo DominikRemo self-requested a review July 21, 2023 09:10
Copy link
Contributor

@Strohgelaender Strohgelaender left a comment

Choose a reason for hiding this comment

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

code looks good to me

Copy link
Contributor

@DominikRemo DominikRemo left a comment

Choose a reason for hiding this comment

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

Code LGTM

@pal03377 pal03377 temporarily deployed to artemis-test5.artemis.cit.tum.de July 21, 2023 15:47 — with GitHub Actions Inactive
Copy link
Contributor

@pal03377 pal03377 left a comment

Choose a reason for hiding this comment

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

Offline mode and errors are working for me after some delay (see video). TS5, exam mode testing session.

I get errors for all operations except for deletion, where I get no feedback at all:
https://github.com/ls1intum/Artemis/assets/9006596/f36a3a46-c431-418a-903f-efcbfe18eb6c

I would be fine with this being solved in a follow-up though => approve ✅
Note: The deletion errors were visible for @aplr in Firefox, but not for me in Chrome.

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.

Other than the issues @pal03377 mentioned, the improvements worked.

Copy link
Contributor

@FelixTJDietrich FelixTJDietrich 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 during the exam testing session. I got errors when not connected to wifi for all operations except the deletion of the file on both Safari and Chrome.

Note that rename and adding a file does explicitly state that the internet connection is bad.

Copy link
Contributor

@basak-akan basak-akan 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 during exam mode testing session. Loading and editing existing files works as described.
However, I don't receive any error for deletion while offline as @pal03377 mentioned.

@krusche krusche changed the title Programming exercises: Improve error messages for connection issues in code editor Programming exercises: Improve error messages for connection issues in online code editor Jul 22, 2023
@krusche krusche merged commit fd5372f into develop Jul 22, 2023
34 of 36 checks passed
@krusche krusche deleted the enhancement/code-editor/improve-offline-errors branch July 22, 2023 06:33
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!) component:Exam Mode component:Programming ready to merge tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.