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

Iris: Fix two UX issues #7115

Merged
merged 8 commits into from
Sep 15, 2023
Merged

Iris: Fix two UX issues #7115

merged 8 commits into from
Sep 15, 2023

Conversation

Hialus
Copy link
Member

@Hialus Hialus commented Aug 30, 2023

Checklist

General

Server

  • Important: I implemented the changes with a very good performance and prevented too many (unnecessary) database calls.
  • I followed the coding and design guidelines.
  • I added multiple integration tests (Spring) related to the features (with a high test coverage).
  • I added pre-authorization annotations according to the guidelines and checked the course groups for all new REST Calls (security).
  • I documented the Java code using JavaDoc style.

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 added authorities to all new routes and checked the course groups for displaying navigation elements (links, buttons).
  • 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

There were three minor issues with the client code of Iris:

  1. The resend feature did not work
  2. If certain errors happened, it was impossible to start a new session
  3. The tests did not pass

Description

This PR fixes all three issues:

  1. Resend now works again in all relevant cases
  2. You can now always start a new session
  3. The tests pass

Steps for Testing

(Don't be scared by the length, they are just very detailed 😉)

Prerequisites:

  • 1 Instructor
  1. Log in to Artemis
  2. Navigate to exercise 1 in course 1 (student view)
  3. Start the exercise (if not already done previously)
  4. Wait for it to be started
  5. Open Iris by clicking on the speech bubble in the bottom right
  6. Send a message, wait for a response -> works
  7. Now close Iris, click the "View" button and then click the "Iris" button
  8. Change the first {{#system~}} to {{system~}}
  9. Go back to the student view
  10. Send a message -> Error occurs
  11. You can see the resend symbol -> click it -> error again obviously
  12. You can start a new session by clicking on the trash can
  13. Send a message -> does not work
  14. Important: Change the {{system~}} to {{#system~}}!
  15. Resend the message -> works

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

Screenshots

@Hialus Hialus self-assigned this Aug 30, 2023
@github-actions github-actions bot added tests server Pull requests that update Java code. (Added Automatically!) client Pull requests that update TypeScript code. (Added Automatically!) labels Aug 30, 2023
@Hialus Hialus marked this pull request as ready for review September 7, 2023 22:31
@Hialus Hialus requested a review from a team as a code owner September 7, 2023 22:31
@RY997 RY997 added the deploy:artemis-test9 Testserver for Project Theia label Sep 12, 2023
@github-actions github-actions bot removed the deploy:artemis-test9 Testserver for Project Theia label Sep 12, 2023
@github-actions
Copy link

❌ Unable to deploy to test servers ❌

Testserver "artemis-test9.artemis.cit.tum.de" is already in use by PR #6931.

@github-actions github-actions bot added the deployment-error Added by deployment workflows if an error occured label Sep 12, 2023
@RY997 RY997 removed the deployment-error Added by deployment workflows if an error occured label Sep 12, 2023
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.

The code overall looks good to me, leaving two small comments.
And will test it after ts9 is released.

src/main/webapp/app/iris/http-message.service.ts Outdated Show resolved Hide resolved
@RY997 RY997 added the deploy:artemis-test9 Testserver for Project Theia label Sep 14, 2023
@RY997 RY997 temporarily deployed to artemis-test9.artemis.cit.tum.de September 14, 2023 08:29 — with GitHub Actions Inactive
@github-actions github-actions bot added lock:artemis-test9 and removed deploy:artemis-test9 Testserver for Project Theia labels Sep 14, 2023
# Conflicts:
#	src/main/webapp/app/iris/exercise-chatbot/exercise-chatwidget/exercise-chat-widget.component.html
@bassner bassner merged commit 4434480 into develop-iris Sep 15, 2023
@bassner bassner deleted the bugfix/iris-ux-fixes branch September 15, 2023 10:14
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:Iris ready for review server Pull requests that update Java code. (Added Automatically!) tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants