-
Notifications
You must be signed in to change notification settings - Fork 301
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
Integrated code lifecycle
: Allow Admins to clear Distributed data
#10111
base: develop
Are you sure you want to change the base?
Integrated code lifecycle
: Allow Admins to clear Distributed data
#10111
Conversation
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 pmd (7.8.0)src/main/java/de/tum/cit/aet/artemis/core/web/admin/AdminBuildJobQueueResource.javaThe following rules are missing or misspelled in your ruleset file category/vm/bestpractices.xml: BooleanInstantiation, DontImportJavaLang, DuplicateImports, EmptyFinallyBlock, EmptyIfStmt, EmptyInitializer, EmptyStatementBlock, EmptyStatementNotInLoop, EmptySwitchStatements, EmptySynchronizedBlock, EmptyTryBlock, EmptyWhileStmt, ExcessiveClassLength, ExcessiveMethodLength, ImportFromSamePackage, MissingBreakInSwitch, SimplifyBooleanAssertion. Please check your ruleset configuration. WalkthroughThis pull request introduces a new feature for clearing distributed data in the build agent system. It includes the addition of a new administrative endpoint in the backend, a corresponding service method, and user interface components in Angular for confirmation. The feature is designed to ensure that only users with admin privileges can execute the operation, with clear warnings about the irreversible nature of clearing distributed data. Changes
Sequence DiagramsequenceDiagram
participant User
participant BuildAgentSummaryComponent
participant BuildAgentClearDistributedDataModal
participant BuildAgentsService
participant AdminBuildJobQueueResource
participant SharedQueueManagementService
User->>BuildAgentSummaryComponent: Click "Clear Distributed Data"
BuildAgentSummaryComponent->>BuildAgentClearDistributedDataModal: Open Modal
User->>BuildAgentClearDistributedDataModal: Confirm by typing "CLEAR DATA"
BuildAgentClearDistributedDataModal->>BuildAgentSummaryComponent: Confirm Action
BuildAgentSummaryComponent->>BuildAgentsService: Call clearDistributedData()
BuildAgentsService->>AdminBuildJobQueueResource: HTTP DELETE /clear-distributed-data
AdminBuildJobQueueResource->>SharedQueueManagementService: Call clearDistributedData()
SharedQueueManagementService-->>AdminBuildJobQueueResource: Clear Completed
AdminBuildJobQueueResource-->>BuildAgentsService: Return Success
BuildAgentsService-->>BuildAgentSummaryComponent: Show Success Alert
Possibly related PRs
Suggested Labels
Suggested Reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this 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
🧹 Nitpick comments (10)
src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-pause-all-modal/build-agent-pause-all-modal.component.ts (2)
1-5
: Use consistent single quotes for imports
While the Angular style guide does not strictly enforce single quotes everywhere, the local coding guidelines mention using single quotes for strings. Consider aligning the import statements accordingly if that’s the current standard.-import { Component, inject } from "@angular/core"; +import { Component, inject } from '@angular/core';
22-27
: Add return types to methods
Even though Angular/TypeScript can infer some return types, explicitly specifying return types (e.g.,void
) can improve clarity.- cancel() { + cancel(): void { this.activeModal.dismiss('cancel'); }src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-clear-distributed-data/build-agent-clear-distributed-data.component.ts (2)
1-7
: Consider adding more descriptive naming
The nameBuildAgentClearDistributedDataComponent
is accurate; however, a shorter alias or more domain-specific naming might improve readability in templates and usage, especially if more admin modals or dialogs are introduced in the future.
22-25
: Use placeholders or safer approach for user confirmation checks
Relying on the user to type "CLEAR DATA" exactly might be error-prone. You may want to provide a placeholder or more robust form validation approach. Also consider localizing the required string if it should be language-dependent.src/test/javascript/spec/component/localci/build-agents/build-agent-clear-distributed-data.component.spec.ts (1)
33-39
: Improve test specificity
You’ve added a test ensuring that the button is disabled unless the user types the exact phrase. Consider adding more cases (e.g., trailing whitespace, localization differences) to ensure robust validation.src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-summary.component.ts (1)
104-110
: Handle modal dismissal
This approach works for positive confirmations. However, consider handling themodalRef.dismiss
scenario to avoid potential issues if the user closes the modal outside the confirm/cancel flow.src/main/java/de/tum/cit/aet/artemis/core/web/admin/AdminBuildJobQueueResource.java (1)
275-291
: HTTP method choice
ThePUT
method is acceptable for this endpoint. However, given it clears data, aDELETE
request might also be semantically suitable. If the team prefersPUT
, ensure documentation clarifies the rationale for the chosen method.src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-clear-distributed-data/build-agent-clear-distributed-data.component.html (3)
1-7
: Enhance modal accessibilityAdd descriptive aria-label to the modal dialog and improve the close button accessibility.
-<div class="modal-content"> +<div class="modal-content" role="dialog" aria-labelledby="clear-data-modal-title"> <div class="modal-header"> - <h5 class="modal-title"> + <h5 class="modal-title" id="clear-data-modal-title"> <span jhiTranslate="artemisApp.buildAgents.clearDistributedData.title"></span> </h5> - <button type="button" class="btn-close" aria-label="Close" (click)="cancel()"></button> + <button type="button" class="btn-close" aria-label="Close modal" (click)="cancel()"></button>
8-15
: Add validation feedback for confirmation textThe input field should display feedback when the entered text doesn't match the required pattern.
-<input type="text" class="form-control" required name="confirm" [(ngModel)]="confirmationText" [pattern]="'CLEAR DATA'" /> +<input + type="text" + class="form-control" + required + name="confirm" + [(ngModel)]="confirmationText" + [pattern]="'CLEAR DATA'" + #confirmInput="ngModel" + aria-label="Confirmation text input" +/> +<div class="invalid-feedback" *ngIf="confirmInput.invalid && (confirmInput.dirty || confirmInput.touched)"> + <span jhiTranslate="artemisApp.buildAgents.clearDistributedData.invalidConfirmation">Please enter 'CLEAR DATA' exactly as shown</span> +</div>
16-26
: Add loading state to prevent double submissionThe danger button should be disabled during the clearing operation to prevent double submissions.
-<button type="button" class="btn btn-danger" (click)="confirm()" [disabled]="!buttonEnabled()"> +<button + type="button" + class="btn btn-danger" + (click)="confirm()" + [disabled]="!buttonEnabled() || isLoading" + aria-label="Confirm clear data"> <fa-icon [icon]="faTrash" /> <span jhiTranslate="artemisApp.buildAgents.clearDistributedData.clearData"></span> + <span *ngIf="isLoading" class="spinner-border spinner-border-sm ms-1" role="status"></span> </button>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/main/java/de/tum/cit/aet/artemis/core/web/admin/AdminBuildJobQueueResource.java
(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/localci/SharedQueueManagementService.java
(4 hunks)src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-clear-distributed-data/build-agent-clear-distributed-data.component.html
(1 hunks)src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-clear-distributed-data/build-agent-clear-distributed-data.component.ts
(1 hunks)src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-pause-all-modal/build-agent-pause-all-modal.component.html
(1 hunks)src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-pause-all-modal/build-agent-pause-all-modal.component.ts
(1 hunks)src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-summary.component.html
(1 hunks)src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-summary.component.ts
(4 hunks)src/main/webapp/app/localci/build-agents/build-agents.service.ts
(1 hunks)src/main/webapp/i18n/de/buildAgents.json
(1 hunks)src/main/webapp/i18n/en/buildAgents.json
(1 hunks)src/test/javascript/spec/component/localci/build-agents/build-agent-clear-distributed-data.component.spec.ts
(1 hunks)src/test/javascript/spec/component/localci/build-agents/build-agent-pause-all-modal.component.spec.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
src/main/webapp/app/localci/build-agents/build-agents.service.ts (1)
src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-clear-distributed-data/build-agent-clear-distributed-data.component.html (1)
Pattern src/main/webapp/**/*.html
: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/java/de/tum/cit/aet/artemis/core/web/admin/AdminBuildJobQueueResource.java (1)
Pattern src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/SharedQueueManagementService.java (1)
Pattern src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-summary.component.html (1)
Pattern src/main/webapp/**/*.html
: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/test/javascript/spec/component/localci/build-agents/build-agent-pause-all-modal.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts
: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-pause-all-modal/build-agent-pause-all-modal.component.html (1)
Pattern src/main/webapp/**/*.html
: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-pause-all-modal/build-agent-pause-all-modal.component.ts (1)
src/test/javascript/spec/component/localci/build-agents/build-agent-clear-distributed-data.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts
: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-clear-distributed-data/build-agent-clear-distributed-data.component.ts (1)
src/main/webapp/i18n/de/buildAgents.json (1)
Pattern src/main/webapp/i18n/de/**/*.json
: German language translations should be informal (dutzen) and should never be formal (sietzen). So the user should always be addressed with "du/dein" and never with "sie/ihr".
src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-summary.component.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: Analyse
🔇 Additional comments (21)
src/test/javascript/spec/component/localci/build-agents/build-agent-pause-all-modal.component.spec.ts (6)
1-2
: Imports look good.
The usage of Angular's testing utilities is aligned with standard practices and there's no non-essential import overhead.
3-6
: Component import is valid.
Referencing theBuildAgentPauseAllModalComponent
directly rather than importing an entire module helps maintain a lean test environment.
7-14
: Consider testingupdate
if it's used in production code.
You've mockedupdate
in theactiveModal
, but there is no test verifying its usage. Confirm if the component callsupdate
, and add a test if needed.
16-25
: Module configuration aligns with guidelines.
MockingNgbActiveModal
prevents actual modal operations while focusing on unit testing this component's logic.
27-31
: Well-defined “cancel” behavior.
The test ensures that the modal’sdismiss
method is called with the expected argument, improving reliability and clarity of the user cancel flow.
33-37
: Confirm logic validated.
The test confirms that the modal’sclose
method is called withtrue
, verifying the expected acceptance flow when confirming the action.src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-pause-all-modal/build-agent-pause-all-modal.component.ts (1)
7-12
: Component configuration is appropriate
Thestandalone: true
approach is a good practice for modularity, and the external template reference is consistent with Angular best practices.src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-clear-distributed-data/build-agent-clear-distributed-data.component.ts (1)
33-35
: Flag potential for accidental data loss
Clearing distributed data can be irreversible. Confirm the user experience and incorporate additional disclaimers or logs to avoid unintentional actions.src/test/javascript/spec/component/localci/build-agents/build-agent-clear-distributed-data.component.spec.ts (2)
1-5
: Test structure aligns with Angular best practices
The usage ofTestBed.configureTestingModule
and explicit import of the component is correct. This is a well-structured foundational test setup.
41-45
: Confirm final user feedback
After closing withtrue
, confirm that the calling component or service handles the result properly (e.g., triggers the clearing logic). You could add a separate test mocking or verifying the subsequent action.src/main/webapp/app/localci/build-agents/build-agents.service.ts (1)
77-83
: HTTP request approach is consistent and clear
Using aDELETE
request is appropriate for data removal. The error handling uses a descriptive message, which is beneficial if issues arise. However, consider logging or further surfacing the specific context for operational troubleshooting.src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-summary.component.ts (4)
6-6
: Imports look good!
These imports align with the Angular style guide, and the icon usage from FontAwesome is consistent with best practices.Also applies to: 10-10, 15-16, 45-45
113-120
: Modal for clearing distributed data
Great job reusing the same pattern as pausing build agents. This enhances maintainability and consistency across modals.
122-122
: Method naming consistency
The namepauseAllBuildAgents()
precisely reflects its purpose and follows the recommended Angular style for method names (camelCase).
158-173
: Concurrency check
Clearing distributed data may conflict if another admin triggers the process simultaneously. While this might be an edge case, consider adding protective measures (e.g., concurrency locks or checks) to ensure safe multi-admin operations.src/main/java/de/tum/cit/aet/artemis/programming/service/localci/SharedQueueManagementService.java (1)
41-41
: Clear data logic
- Initializing
resultQueue
and clearing it inclearDistributedData()
looks correct.- Ensure you call
cancelAllRunningBuildJobs()
before clearingprocessingJobs
to handle active jobs gracefully.- Using Hazelcast’s
IQueue.clear()
is fine for an admin-only operation, but confirm that no partial states remain if the operation is unexpectedly interrupted.Also applies to: 65-66, 106-106, 313-325
✅ Verification successful
Implementation correctly handles clearing of distributed data
The code follows best practices by:
- Calling
cancelAllRunningBuildJobs()
before clearingprocessingJobs
as recommended- Using atomic clear operations that prevent partial states
- Implementing proper access controls and user confirmation for this admin-only operation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Look for transaction-like patterns or partial rollback to confirm there's no partial states left on interruption rg -A 5 "transaction|rollback"Length of output: 66119
Script:
#!/bin/bash # Look for clearDistributedData implementation and surrounding context ast-grep --pattern 'clearDistributedData() { $$$ }' # Backup search using ripgrep rg -A 10 "clearDistributedData"Length of output: 16011
src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-pause-all-modal/build-agent-pause-all-modal.component.html (1)
1-23
: Modal structure
The modal follows Angular and Bootstrap best practices:
• Translations for text content.
• Separate confirm and cancel buttons.
• Use of FontAwesome icons for clarity.
All good!src/main/webapp/i18n/en/buildAgents.json (2)
28-29
: LGTM! Clear and informative alert messagesThe alert messages effectively communicate the success and failure states of the operation.
36-42
: LGTM! Well-structured warning messagesThe clear data section provides comprehensive information about:
- What will be cleared
- The irreversible nature of the action
- Clear instructions for proceeding
src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-summary.component.html (2)
Line range hint
2-2
: Great use of modern Angular control flow syntax!The template consistently uses the new
@if
,@for
, and@switch
syntax throughout, following the coding guidelines.Also applies to: 2-2
16-23
: LGTM! Button changes follow consistent patterns.The modifications maintain UI consistency with other administrative actions. The new clear distributed data button is well-integrated with appropriate styling and iconography.
Let's verify the internationalization key exists:
✅ Verification successful
Internationalization keys are properly implemented
The translation keys for the clear distributed data feature are correctly implemented in both English and German language files, including all necessary UI text elements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the i18n key exists in both English and German translations rg -l "artemisApp.buildAgents.clearDistributedData.title" src/main/webapp/i18n/Length of output: 79
Script:
#!/bin/bash # First, let's find all i18n JSON files fd -e json . src/main/webapp/i18n/ # Then, let's look for any build agents related keys rg -i "buildAgents" src/main/webapp/i18n/ --type json # Also search for clearDistributed to catch any variations of the key rg -i "clearDistributed" src/main/webapp/i18n/ --type jsonLength of output: 15460
Script:
#!/bin/bash # Check the content of the clearDistributedData section in both language files rg -A 5 '"clearDistributedData"' src/main/webapp/i18n/*/buildAgents.jsonLength of output: 2030
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, added two minors regarding the translations
|
|
There was a problem hiding this 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
🧹 Nitpick comments (2)
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/SharedQueueManagementService.java (2)
106-106
: Add documentation for the resultQueue field.While the initialization is correct, the purpose and contents of this queue should be documented in the class-level or field-level JavaDoc, similar to how
processingJobs
is documented.+ /** + * Queue storing build results that need to be processed + */ private IQueue<ResultQueueItem> resultQueue;
313-316
: Enhance method documentation.The JavaDoc should clearly document:
- The impact of clearing each data structure
- Warning about data loss (especially for non-processed results)
- Prerequisites (e.g., pausing agents)
- Admin-only access requirement
/** * Clear all build related data from the distributed data structures. * This method should only be called by an admin user. + * + * WARNING: This operation: + * - Clears all queued build jobs + * - Removes all processing job information + * - Clears docker image cleanup information + * - Discards all non-processed results + * - Removes all build agent information + * + * Prerequisites: + * - All build agents should be paused before calling this method + * - Only administrators should have access to this operation */
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/SharedQueueManagementService.java
(4 hunks)src/main/webapp/i18n/de/buildAgents.json
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/SharedQueueManagementService.java (1)
Pattern src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/i18n/de/buildAgents.json (1)
Pattern src/main/webapp/i18n/de/**/*.json
: German language translations should be informal (dutzen) and should never be formal (sietzen). So the user should always be addressed with "du/dein" and never with "sie/ihr".
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: client-tests-selected
- GitHub Check: client-style
- GitHub Check: server-style
- GitHub Check: server-tests
- GitHub Check: client-tests
- GitHub Check: Build and Push Docker Image
- GitHub Check: Build .war artifact
- GitHub Check: Analyse
🔇 Additional comments (6)
src/main/webapp/i18n/de/buildAgents.json (4)
27-29
: LGTM! Alert messages are clear and consistent.The new alert messages follow the existing pattern and maintain a consistent informal tone.
35-35
: LGTM! Warning message uses correct informal style.The warning message correctly uses the informal "du" form and clearly explains the consequences of the action.
38-38
: Improve technical terminology in the German translation.The description uses technical terms like "Map" and "Queue" which might be unclear to administrators. Consider using more user-friendly terms to explain these concepts.
Suggestion:
- "descriptionFirst": "Diese Aktion wird alle verteilten Daten löschen. Dazu gehören die BuildJob Queue, die processingJobs Map, die DockerImageCleanUp Map, die BuildAgents Map und die BuildJobResults Queue.", + "descriptionFirst": "Diese Aktion wird alle verteilten Daten löschen. Dazu gehören die Warteschlange der Build-Aufträge, die Liste der in Bearbeitung befindlichen Aufträge, die Liste der Docker-Images zur Bereinigung, die Liste der Build-Agenten und die Warteschlange der Build-Ergebnisse.",
39-42
: LGTM! Clear and user-friendly instructions.The warning about irreversibility and the confirmation instructions are well-written and use the correct informal tone.
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/SharedQueueManagementService.java (2)
41-41
: LGTM!The import statement follows the coding guidelines and is correctly placed with other DTO imports.
65-66
: LGTM!The field declaration follows the coding guidelines:
- Uses appropriate access level (private)
- Consistent with other queue declarations
- Well-placed with related fields
...in/java/de/tum/cit/aet/artemis/programming/service/localci/SharedQueueManagementService.java
Show resolved
Hide resolved
…e/clear-distributed-data' into feature/integrated-code-lifecycle/clear-distributed-data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Demonstrated during testing session, works as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on TS3 during tested session. Works as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worked during testing, code looks good too. I added dome minor change requests
...summary/build-agent-clear-distributed-data/build-agent-clear-distributed-data.component.html
Show resolved
Hide resolved
...t-summary/build-agent-clear-distributed-data/build-agent-clear-distributed-data.component.ts
Show resolved
Hide resolved
export class BuildAgentClearDistributedDataComponent { | ||
private activeModal = inject(NgbActiveModal); | ||
|
||
confirmationText = model<string>(''); | ||
|
||
protected readonly faTimes = faTimes; | ||
protected readonly faTrash = faTrash; | ||
|
||
private readonly expectedConfirmationText = 'CLEAR DATA'; | ||
|
||
buttonEnabled = computed(() => this.confirmationText() === this.expectedConfirmationText); | ||
|
||
/** | ||
* Closes the modal by dismissing it | ||
*/ | ||
cancel() { | ||
this.activeModal.dismiss('cancel'); | ||
} | ||
|
||
confirm() { | ||
this.activeModal.close(true); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not entirely sure, but would it also just work to use a jhi-delete-button instead of this whole component?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested on TS3, works as expected
Checklist
General
Server
Client
authorities
to all new routes and checked the course groups for displaying navigation elements (links, buttons).Changes affecting Programming Exercises
Motivation and Context
We want to allow admin to clear distributed data. This will clear the following structures:
Steps for Testing
Prerequisites:
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
Code Review
Manual Tests
Performance Tests
Screenshots
Summary by CodeRabbit
New Features
User Interface
Localization
Testing