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.

Sign up for GitHub

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

Jump to bottom

Development: Remove deprecated file upload endpoint #7453

Merged
merged 7 commits into from
Oct 28, 2023
Merged

Development: Remove deprecated file upload endpoint #7453

merged 7 commits into from
Oct 28, 2023

Conversation

julian-christl
Copy link
Member

@julian-christl julian-christl commented Oct 26, 2023 •
edited
Loading

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

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.

Motivation and Context

After replacing all uploads with multipart uploads, these two endpoints are not used anymore.

Description

I removed the two endpoints from the server and all related tests. I also removed the endpoint from the client and simplified the corresponding service. I added some checks for the file retrieval to keep the test coverage stable.

Testserver States

Note

These badges show the state of the test servers.
Green = Currently available, Red = Currently locked







Review Progress

Confirm that the endpoints are not in use anymore.

Code Review

  • Code Review 1
  • Code Review 2

@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!) cypress Pull requests that update cypress tests. (Added Automatically!) labels Oct 26, 2023
@julian-christl julian-christl changed the base branch from develop to feature/reimplement-quiz-image-upload October 26, 2023 17:46
@julian-christl julian-christl force-pushed the chore/remove-deprecated-file-upload branch from 6161182 to ca57739 Compare October 26, 2023 17:55
@github-actions github-actions bot removed the cypress Pull requests that update cypress tests. (Added Automatically!) label Oct 26, 2023
Base automatically changed from feature/reimplement-quiz-image-upload to develop October 27, 2023 18:49
@julian-christl julian-christl force-pushed the chore/remove-deprecated-file-upload branch from ca57739 to 7dec68e Compare October 27, 2023 19:10
@julian-christl julian-christl marked this pull request as ready for review October 27, 2023 19:12
@julian-christl julian-christl requested a review from a team as a code owner October 27, 2023 19:12
JohannesStoehr
JohannesStoehr previously approved these changes Oct 27, 2023
View reviewed changes
Copy link
Contributor

@JohannesStoehr JohannesStoehr left a comment

Choose a reason for hiding this comment

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

Removal looks good to me. Left one optional comment

src/main/java/de/tum/in/www1/artemis/web/rest/FileResource.java Outdated Show resolved Hide resolved
RY997
RY997 previously approved these changes Oct 28, 2023
View reviewed changes
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.

LGTM

JohannesStoehr
JohannesStoehr previously approved these changes Oct 28, 2023
View reviewed changes
Copy link
Contributor

@JohannesStoehr JohannesStoehr 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 even better now

@julian-christl julian-christl dismissed stale reviews from JohannesStoehr and RY997 via 2c85bc7 October 28, 2023 15:39
@julian-christl
Copy link
Member Author

Thanks, I forgot to remove the redundant null-check, though.

Strohgelaender
Strohgelaender approved these changes Oct 28, 2023
View reviewed changes
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

RY997
RY997 approved these changes Oct 28, 2023
View reviewed changes
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.

Reapprove!

@julian-christl julian-christl added this to the 6.6.4 milestone Oct 28, 2023
@krusche krusche changed the title Development: Removed deprecated file upload endpoint Development: Remove deprecated file upload endpoint Oct 28, 2023
@krusche krusche modified the milestones: 6.6.4, 6.6.3 Oct 28, 2023
@krusche krusche merged commit 475754f into develop Oct 28, 2023
32 of 35 checks passed
@krusche krusche deleted the chore/remove-deprecated-file-upload branch October 28, 2023 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RY997 RY997 RY997 approved these changes

@JohannesStoehr JohannesStoehr JohannesStoehr approved these changes

@Strohgelaender Strohgelaender Strohgelaender approved these changes

Assignees

@julian-christl julian-christl

Labels
client Pull requests that update TypeScript code. (Added Automatically!) ready to merge server Pull requests that update Java code. (Added Automatically!) tests
Projects
None yet
Milestone
6.6.3
Development

Successfully merging this pull request may close these issues.
5 participants
@julian-christl @Strohgelaender @JohannesStoehr @RY997 @krusche