`Development`: Ensure all endpoints return response entities #8358

MaximilianAnzinger · 2024-04-09T09:42:00Z

Checklist

General

I chose a title conforming to the naming conventions for pull requests.

Server

I strictly followed the server coding and design guidelines.
I documented the Java code using JavaDoc style.

Motivation and Context

Follow up for #8344 to enable the missing check and migrate our endpoints that violate the rule.

Code Review

Code Review 1
Code Review 2

add arch test add integration test and refactor TutorialGroup code fix ArchCondition name refactor architecture tests and add more move StudentExamRepository getIndividualDueDate to ExerciseDateServiceual

…ResponseEntity-check

coderabbitai · 2024-04-09T15:37:42Z

Walkthrough

The recent updates focus on refining API response handling and improving code organization in the Artemis project. Changes include the adoption of ResponseEntity for method return types to enhance API endpoint responses and the inclusion of specific imports to streamline code readability and functionality. These modifications aim to elevate the maintainability and clarity of the codebase.

Changes

File Path	Change Summary
`.../web/rest/ExamResource.java`	Added imports; Changed methods to return `ResponseEntity`; Enhanced API response handling.
`.../web/rest/ogparser/LinkPreviewResource.java`	Updated with HTTP method annotations, added controller mapping, and switched method return type to `ResponseEntity`.

Recent Review Details

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 25a96b6 and 0741cdd.

Files selected for processing (1)

src/main/java/de/tum/in/www1/artemis/web/rest/ogparser/LinkPreviewResource.java (1 hunks)

Files skipped from review as they are similar to previous changes (1)

src/main/java/de/tum/in/www1/artemis/web/rest/ogparser/LinkPreviewResource.java

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 4

Actionable comments outside the diff hunks (3)

src/main/java/de/tum/in/www1/artemis/service/scheduled/ProgrammingExerciseScheduleService.java (1)

Line range hint 92-117: Consider decomposing the class to adhere to the Single Responsibility Principle.

The constructor injects a large number of dependencies, indicating that the class may have multiple responsibilities. Consider breaking down this service into smaller, more focused services, each handling a specific aspect of programming exercise scheduling. This can enhance maintainability and testability.

src/main/java/de/tum/in/www1/artemis/web/rest/CourseResource.java (2)

Line range hint 158-158: Refactor updateCourse to reduce its complexity by extracting logic into smaller methods.

Line range hint 158-158: Consider using a service layer to encapsulate business logic in updateCourse, separating it from the web layer.

src/main/java/de/tum/in/www1/artemis/web/rest/ExamResource.java

src/main/java/de/tum/in/www1/artemis/web/rest/ogparser/LinkPreviewResource.java

src/main/java/de/tum/in/www1/artemis/service/tutorialgroups/TutorialGroupScheduleService.java

Jan-Thurner

Code LGTM 👍

…icMethodsShouldReturnResponseEntity-check

coderabbitai

Actionable comments posted: 1

src/test/java/de/tum/in/www1/artemis/architecture/ArchitectureTest.java

…ResponseEntity-check

b-fein · 2024-04-09T19:57:37Z

I’d like to understand why for example returning a ResponseEntity<OnlineResourceDTO> is better than returning OnlineResourceDTO from a REST method. Can you please extend the PR description to explain/motivate why this change is useful?

MaximilianAnzinger · 2024-04-10T12:20:40Z

@b-fein
We discussed this and felt that consistency across all endpoints would be beneficial for the maintenance of this project. Most of our endpoints already apply this rule.
This way it's easier for new developers to distinguish between service methods and endpoints and it makes it clear that endpoints should not be used as service methods. (Yes, this is something we encountered in the past.)

…ResponseEntity-check

Jan-Thurner

reapprove

src/main/java/de/tum/in/www1/artemis/web/rest/ogparser/LinkPreviewResource.java

Co-authored-by: Raphael Stief <118574504+rstief@users.noreply.github.com>

rstief

code lgtm 👍

refactor architecture tests and add more

823e1d0

add arch test add integration test and refactor TutorialGroup code fix ArchCondition name refactor architecture tests and add more move StudentExamRepository getIndividualDueDate to ExerciseDateServiceual

github-actions bot added tests server Pull requests that update Java code. (Added Automatically!) labels Apr 9, 2024

github-actions bot assigned MaximilianAnzinger Apr 9, 2024

MaximilianAnzinger mentioned this pull request Apr 9, 2024

Development: Enhance architecture tests #8344

Merged

5 tasks

MaximilianAnzinger and others added 7 commits April 9, 2024 14:44

enable check and fix endpoints

4f2614f

check shouldBeNamedRepository rule

406f0a6

fix ResponseEntity of register endpoint

f07b41f

Merge branch 'develop' into chore/add-architecture-tests

e5b1b66

fix compilation error

a85b300

exlude special case LinkPreviewResource

6cd0b9d

Merge branch 'develop' into chore/enable-allPublicMethodsShouldReturn…

8e36f2f

…ResponseEntity-check

MaximilianAnzinger added the stacked-pr PR that depends on another PR label Apr 9, 2024

MaximilianAnzinger marked this pull request as ready for review April 9, 2024 15:37

MaximilianAnzinger requested a review from a team as a code owner April 9, 2024 15:37

github-actions bot added the ready for review label Apr 9, 2024

coderabbitai bot requested changes Apr 9, 2024

View reviewed changes

coderabbitai bot previously approved these changes Apr 9, 2024

View reviewed changes

MaximilianAnzinger changed the title ~~Development: Enable allPublicMethodsShouldReturnResponseEntity check~~ Development: Enable ResponseEntity architecture test Apr 9, 2024

MaximilianAnzinger changed the title ~~Development: Enable ResponseEntity architecture test~~ Development: Ensure all endpoints return ResponseEntities Apr 9, 2024

Jan-Thurner previously approved these changes Apr 9, 2024

View reviewed changes

Merge branch 'chore/add-architecture-tests' into chore/enable-allPubl…

d5d1f17

…icMethodsShouldReturnResponseEntity-check

MaximilianAnzinger dismissed stale reviews from Jan-Thurner and coderabbitai via d5d1f17 April 9, 2024 18:36

MaximilianAnzinger added the blocked label Apr 9, 2024

MaximilianAnzinger added this to the 7.0.0 milestone Apr 9, 2024

coderabbitai bot requested changes Apr 9, 2024

View reviewed changes

src/test/java/de/tum/in/www1/artemis/architecture/ArchitectureTest.java Show resolved Hide resolved

MaximilianAnzinger removed blocked stacked-pr PR that depends on another PR labels Apr 9, 2024

Merge branch 'develop' into chore/enable-allPublicMethodsShouldReturn…

5ac0752

…ResponseEntity-check

add exception for ResourceArchitectureTest

187eccb

Merge branch 'develop' into chore/enable-allPublicMethodsShouldReturn…

fe4859b

…ResponseEntity-check

coderabbitai bot approved these changes Apr 10, 2024

View reviewed changes

coderabbitai bot previously approved these changes Apr 10, 2024

View reviewed changes

fix import

25a96b6

MaximilianAnzinger dismissed coderabbitai’s stale review via 25a96b6 April 10, 2024 12:38

MaximilianAnzinger requested review from Jan-Thurner and rstief April 10, 2024 13:39

Jan-Thurner previously approved these changes Apr 10, 2024

View reviewed changes

rstief previously approved these changes Apr 10, 2024

View reviewed changes

src/main/java/de/tum/in/www1/artemis/web/rest/ogparser/LinkPreviewResource.java Outdated Show resolved Hide resolved

fix LinkPreviewResource JavaDoc

0741cdd

Co-authored-by: Raphael Stief <118574504+rstief@users.noreply.github.com>

MaximilianAnzinger dismissed stale reviews from rstief and Jan-Thurner via 0741cdd April 10, 2024 14:01

rstief approved these changes Apr 10, 2024

View reviewed changes

Jan-Thurner approved these changes Apr 10, 2024

View reviewed changes

krusche changed the title ~~Development: Ensure all endpoints return ResponseEntities~~ Development: Ensure all endpoints return response entities Apr 10, 2024

krusche merged commit 265c399 into develop Apr 10, 2024
21 of 27 checks passed

krusche deleted the chore/enable-allPublicMethodsShouldReturnResponseEntity-check branch April 10, 2024 20:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`Development`: Ensure all endpoints return response entities #8358

`Development`: Ensure all endpoints return response entities #8358

MaximilianAnzinger commented Apr 9, 2024 •

edited

coderabbitai bot commented Apr 9, 2024 •

edited

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

Documentation and Community

coderabbitai bot left a comment

Jan-Thurner left a comment

coderabbitai bot left a comment

b-fein commented Apr 9, 2024

MaximilianAnzinger commented Apr 10, 2024

Jan-Thurner left a comment

rstief left a comment

Development: Ensure all endpoints return response entities #8358

Development: Ensure all endpoints return response entities #8358

Conversation

MaximilianAnzinger commented Apr 9, 2024 • edited

Checklist

General

Server

Motivation and Context

Code Review

coderabbitai bot commented Apr 9, 2024 • edited

Walkthrough

Changes

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

Documentation and Community

coderabbitai bot left a comment

Choose a reason for hiding this comment

Jan-Thurner left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

b-fein commented Apr 9, 2024

MaximilianAnzinger commented Apr 10, 2024

Jan-Thurner left a comment

Choose a reason for hiding this comment

rstief left a comment

Choose a reason for hiding this comment

`Development`: Ensure all endpoints return response entities #8358

`Development`: Ensure all endpoints return response entities #8358

MaximilianAnzinger commented Apr 9, 2024 •

edited

coderabbitai bot commented Apr 9, 2024 •

edited

CodeRabbit Configration File (`.coderabbit.yaml`)