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: Theia integration test #9759

Open
wants to merge 69 commits into
base: develop
Choose a base branch
from

Conversation

janthoXO
Copy link
Contributor

@janthoXO janthoXO commented Nov 12, 2024

THIS PR SHOULD NOT BE MERGED AND IS ONLY FOR TESTING PURPOSES

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.







Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced AllowedTools annotation for method-level access control based on tool permissions.
    • Added ToolTokenType enumeration to represent different tool types.
    • Implemented ToolsInterceptor to manage access control using JWT tokens.
    • Enhanced JWT handling with new methods for token extraction and validation.
    • New endpoint to retrieve programming exercises by project key with access restrictions.
    • Added getToolToken method for generating tool-specific bearer tokens in AccountService.
    • Introduced getBuildConfig method in ProgrammingExerciseService to retrieve build configurations.
  • Enhancements

    • Improved error handling and validation in various resource methods.
    • Detailed logging added to course-related methods for better performance tracking.
    • Enhanced participation management logic with stricter adherence to exercise rules and user permissions.
    • Simplified button rendering logic for programming exercises by removing the online IDE button.
    • Updated the authorize method to return an access token in a more structured format.
  • Tests

    • Expanded test coverage for JWT filter functionality with additional test cases.
    • Updated tests to validate the visibility of buttons based on the new access control logic.
    • Enhanced test suite for CodeButtonComponent to include scenarios for Theia integration and button visibility.

@coderabbitai pause

iyannsch and others added 30 commits September 23, 2024 18:17
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@iyannsch iyannsch added the deploy:artemis-test9 Testserver for Project Theia label Nov 13, 2024
@iyannsch iyannsch temporarily deployed to artemis-test9.artemis.cit.tum.de November 13, 2024 14:11 — with GitHub Actions Inactive
@github-actions github-actions bot added lock:artemis-test9 and removed deploy:artemis-test9 Testserver for Project Theia labels Nov 13, 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: 1

🧹 Outside diff range and nitpick comments (1)
src/main/webapp/app/shared/components/code-button/code-button.component.ts (1)

374-380: Simplify artemisUrl construction using window.location.origin

The current URL construction is verbose and potentially error-prone. Using window.location.origin would be more concise and reliable.

-let artemisUrl: string = '';
-if (window.location.protocol) {
-    artemisUrl += window.location.protocol + '//';
-}
-if (window.location.host) {
-    artemisUrl += window.location.host;
-}
+const artemisUrl: string = window.location.origin;
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 3c596ac and 590fde4.

📒 Files selected for processing (1)
  • src/main/webapp/app/shared/components/code-button/code-button.component.ts (6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/app/shared/components/code-button/code-button.component.ts (1)

Pattern src/main/webapp/**/*.ts: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalCase;funcs:camelCase;props:camelCase;no_priv_prefix:true;strings:single_quotes;localize:true;btns:functionality;links:navigation;icons_text:newline;labels:associate;code_style:arrow_funcs,curly_braces,open_braces_same_line,indent_4;memory_leak_prevention:true;routes:naming_schema;chart_framework:ngx-charts;responsive_layout:true

🔇 Additional comments (3)
src/main/webapp/app/shared/components/code-button/code-button.component.ts (3)

13-13: LGTM: Clean import additions and property declarations

The new imports and property declarations are well-organized and follow Angular naming conventions.

Also applies to: 16-16, 19-20, 70-72, 76-76


87-87: LGTM: Proper dependency injection

The ProgrammingExerciseService is correctly injected following Angular's dependency injection pattern.


353-368: Consider potential race condition in Theia initialization

The initialization of Theia settings depends on both the profile info and the build config. If the exercise changes after the build config is retrieved but before the settings are applied, it could lead to inconsistent state.

Consider wrapping the initialization in a check:

 initTheia(profileInfo: ProfileInfo) {
     if (profileInfo.activeProfiles?.includes(PROFILE_THEIA) && this.exercise) {
         this.programmingExerciseService.getBuildConfig(this.exercise.id!).subscribe((buildConfig) => {
+            // Verify exercise hasn't changed during async operation
+            if (this.exercise?.id !== buildConfig.exerciseId) {
+                return;
+            }
             if (this.exercise) {
                 this.exercise.buildConfig = buildConfig;
                 // Rest of the code...
             }
         });
     }
 }

@iyannsch iyannsch added the deploy:artemis-test9 Testserver for Project Theia label Nov 13, 2024
@iyannsch iyannsch temporarily deployed to artemis-test9.artemis.cit.tum.de November 13, 2024 15:49 — with GitHub Actions Inactive
@github-actions github-actions bot added lock:artemis-test9 and removed deploy:artemis-test9 Testserver for Project Theia labels Nov 13, 2024
…ogramming-exercises/provide-theia-clone-information-on-redirect
…mation-on-redirect' into chore/theia-integration-test
@iyannsch
Copy link
Contributor

@coderabbitai pause

coderabbitai[bot]
coderabbitai bot previously approved these changes Nov 13, 2024
@iyannsch iyannsch added the deploy:artemis-test9 Testserver for Project Theia label Nov 13, 2024
@iyannsch iyannsch temporarily deployed to artemis-test9.artemis.cit.tum.de November 13, 2024 16:31 — with GitHub Actions Inactive
@github-actions github-actions bot added lock:artemis-test9 and removed deploy:artemis-test9 Testserver for Project Theia labels Nov 13, 2024
…ogramming-exercises/provide-theia-clone-information-on-redirect
…mation-on-redirect' into chore/theia-integration-test
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!) config-change Pull requests that change the config in a way that they require a deployment via Ansible. core Pull requests that affect the corresponding module exercise Pull requests that affect the corresponding module lock:artemis-test9 programming Pull requests that affect the corresponding module server Pull requests that update Java code. (Added Automatically!) tests
Projects
Status: Ready For Review
Development

Successfully merging this pull request may close these issues.

2 participants