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

Development: Extract client duplicates into re-usable components #7242

Merged
merged 25 commits into from
Nov 28, 2023

Conversation

aplr
Copy link
Contributor

@aplr aplr commented Sep 21, 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.
  • 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 translated all newly inserted strings into English and German.

Motivation and Context

While implementing new features in Artemis, components have been duplicated. This PR extracts those duplicate components into reusable ones and replaces the original implementations.

Description

The following components are affected:

confirm-entity-name

  • Replaced inline-implementation in delete-dialog component
  • Replaced inline-implementation in data-export-confirmation component
  • Replaced inline-implementation in reset-repo-button component

working-time-control

  • Replaced the inline-implementation in student-exam-detail component

Steps for Testing

Delete dialog

Prerequisites:

  • 1 Instructor
  • Some resource to delete
  1. Log in to Artemis
  2. Navigate to Course Administration
  3. Open a course, exam, ...
  4. Check if the delete dialog still works

Exam working time change

Prerequisites:

  • 1 Instructor
  • 1 Exam
  1. Log in to Artemis
  2. Navigate to Exam management
  3. Check if you can still edit the working time w/ the working time dialog

Student exam working time change

Prerequisites:

  • 1 Instructor
  • 1 Exam
  1. Log in to Artemis
  2. Navigate to Exam management
  3. Open the student exam details
  4. Check if you can still edit the individual student exam working time, both absolute and relative

Review Progress

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Test Coverage

Screenshots

(no changes, just refactoring)

@github-actions github-actions bot added the client Pull requests that update TypeScript code. (Added Automatically!) label Sep 21, 2023
@aplr aplr changed the title Refactor reuse of working time control and entity name confirmation [Development] Refactor reuse of working time control and entity name confirmation Sep 21, 2023
@aplr aplr changed the title [Development] Refactor reuse of working time control and entity name confirmation Development: Extract duplicate code into re-usable components Sep 21, 2023
@aplr aplr changed the title Development: Extract duplicate code into re-usable components Development: Extract duplicates into re-usable components Sep 21, 2023
@github-actions github-actions bot added the tests label Sep 22, 2023
@Strohgelaender
Copy link
Contributor

Is this PR ready to review? It has the label but is still in draft state.

@aplr aplr marked this pull request as ready for review September 23, 2023 20:22
@aplr aplr requested a review from a team as a code owner September 23, 2023 20:22
@aplr
Copy link
Contributor Author

aplr commented Sep 23, 2023

@Strohgelaender yes it is forgot to put it out of draft mode

@milljoniaer milljoniaer temporarily deployed to artemis-test4.artemis.cit.tum.de October 5, 2023 11:55 — with GitHub Actions Inactive
milljoniaer
milljoniaer previously approved these changes Oct 5, 2023
Copy link
Contributor

@milljoniaer milljoniaer 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

  • Course, Exam, Exercise Creation and Deletion
  • Exam Time change in general and for specific students

works as expected

Gusti2010
Gusti2010 previously approved these changes Nov 15, 2023
Copy link

@Gusti2010 Gusti2010 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_Legacy and worked as expected!
Ts11_Legacy could not create exams for the students. However this is not related to this PR.

@krusche krusche modified the milestones: 6.6.7, 6.7.0 Nov 16, 2023
Copy link

There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.

@krusche krusche modified the milestones: 6.7.0, 6.7.1 Nov 25, 2023
@github-actions github-actions bot removed the stale label Nov 26, 2023
Copy link
Contributor

@reschandreas reschandreas left a comment

Choose a reason for hiding this comment

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

reapprove after code change

@aplr aplr temporarily deployed to artemis-test5.artemis.cit.tum.de November 27, 2023 18:15 — with GitHub Actions Inactive
@krusche krusche changed the title Development: Extract duplicates into re-usable components Development: Extract client duplicates into re-usable components Nov 27, 2023
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!) cypress Pull requests that update cypress tests. (Added Automatically!) ready to merge tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.