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: Add build config entity for programming exercises #8897

Merged

Conversation

BBesrour
Copy link
Member

@BBesrour BBesrour commented Jun 28, 2024

🚨🚨🚨USE ONLY TS3 DO NOT DEPLOY TO TEST SERVERS 🚨🚨🚨

Checklist

General

Server

  • Important: I implemented the changes with a very good performance and prevented too many (unnecessary) database calls.
  • I strictly followed the server coding and design guidelines.

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 strictly followed the client coding and design guidelines.

Changes affecting Programming Exercises

  • High priority: I tested all changes and their related features with all corresponding user types on a test server configured with the integrated lifecycle setup (LocalVC and LocalCI).
  • I tested all changes and their related features with all corresponding user types on a test server configured with Gitlab and Jenkins.

Motivation and Context

To reduce database queries, we want to move some attributes from ProgrammingExercise to a new entity, ProgrammingExerciseBuildConfig. This is the first PR for this issue #8887

Steps for Testing

PLEASE DON'T DEPLOY TO TEST SERVERS UNTIL CODE REVIEWS ARE DONE
There are no particular testing steps, please test everything that comes to mind that's related to programming exercises. And please specify what you tested in a comment
General testing steps:

  • Check that you can submit programming exercises and that it works as expected.
  • Try editing a programming exercise, please change the values relevant to the build, like buildscript, sequential tests, etc make sure that the values are saved correctly
  • Import an exercise (normal import within Artemis)
  • Export exercise
  • Import an exercise from file
  • Repeat steps for exam

Testserver States

Note

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







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

Summary by CodeRabbit

  • New Features

    • Introduced a dedicated configuration entity for programming exercises, enhancing management of build settings.
    • Enhanced data retrieval methods to include associated build configurations, improving the completeness of programming exercise data.
  • Bug Fixes

    • Improved handling of build configurations to ensure they are saved and retrieved correctly across various services and tests.
  • Tests

    • Updated multiple test cases to incorporate and validate the new build configuration management, ensuring comprehensive testing of programming exercises.
    • Refactored tests to utilize build configurations in assertions and setup processes, improving test reliability and accuracy.

@BBesrour BBesrour self-assigned this Jun 28, 2024
@github-actions github-actions bot added tests server Pull requests that update Java code. (Added Automatically!) database Pull requests that update the database. (Added Automatically!). Require a CRITICAL deployment. labels Jun 28, 2024
Copy link

github-actions bot commented Jul 5, 2024

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.

@github-actions github-actions bot added the stale label Jul 5, 2024
@BBesrour BBesrour removed the stale label Jul 7, 2024
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 34

Outside diff range comments (1)
src/test/java/de/tum/in/www1/artemis/exercise/programming/ProgrammingExerciseFactory.java (1)

Line range hint 124-142: Encapsulation Improvement Suggestion

Consider encapsulating build configuration setup within ProgrammingExerciseBuildConfig to promote modularity and reduce direct manipulation of its properties.

public class ProgrammingExerciseBuildConfig {
    public void initializeDefaults() {
        this.setTestwiseCoverageEnabled(false);
        this.setBuildScript("Some script");
        this.setBuildPlanConfiguration("{\"api\":\"v0.0.1\",\"metadata\":{},\"actions\":[]}");
        this.setBranch(DEFAULT_BRANCH);
    }
}

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 19

coderabbitai[bot]
coderabbitai bot previously approved these changes Aug 11, 2024
JohannesStoehr
JohannesStoehr previously approved these changes Aug 11, 2024
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

…cle/build-config-instructors

# Conflicts:
#	src/test/java/de/tum/in/www1/artemis/competency/CompetencyIntegrationTest.java
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 29

Outside diff range comments (1)
src/main/java/de/tum/in/www1/artemis/service/connectors/gitlabci/GitLabCIService.java (1)

Line range hint 173-173: Consider addressing TODO comments for variable management.

The TODOs suggest improvements in managing variables at the group level and updating existing ones.

Would you like assistance in implementing these improvements or opening a GitHub issue to track them?

coderabbitai[bot]
coderabbitai bot previously approved these changes Aug 11, 2024
coderabbitai[bot]

This comment was marked as outdated.

coderabbitai[bot]

This comment was marked as outdated.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 26

@krusche krusche merged commit 1b4a050 into develop Aug 11, 2024
39 of 43 checks passed
@krusche krusche deleted the feature/integrated-code-lifecycle/build-config-instructors branch August 11, 2024 21:41
@krusche krusche changed the title Integrated code lifecycle: Add BuildConfig entity Development: Add build config entity for programming exercises Aug 11, 2024
iyannsch added a commit that referenced this pull request Aug 12, 2024
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!) database Pull requests that update the database. (Added Automatically!). Require a CRITICAL deployment. ready for review ready to merge server Pull requests that update Java code. (Added Automatically!) tests
Projects
Archived in project