Development: Utilize server endpoint for getting longest working time in exam assessment
#8749
Conversation
WalkthroughThe Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant ExamAssessmentButtonsComponent
participant Server
User ->> ExamAssessmentButtonsComponent: Load Component
ExamAssessmentButtonsComponent ->> Server: getLongestWorkingTimeForExam
Server -->> ExamAssessmentButtonsComponent: Return Longest Working Time
ExamAssessmentButtonsComponent ->> ExamAssessmentButtonsComponent: Set Exam Data & Recalculate Status
ExamAssessmentButtonsComponent -->> User: Display Updated Exam Status
Assessment against linked issues
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Additional context usedPath-based instructions (1)
Biome
Additional comments not posted (1)
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
github-actions
bot
added
tests
client
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range comments (4)
src/main/webapp/app/course/dashboards/assessment-dashboard/exam-assessment-buttons/exam-assessment-buttons.component.ts (2)
Line range hint
136-136: What's this? Implicit any? Are you trying to write JavaScript in TypeScript? Specify the type explicitly to avoid 'any' accidents.- let errorDetail; + let errorDetail: string | undefined;Tools
Biome
[error] 65-65: Forbidden non-null assertion. (lint/style/noNonNullAssertion)
[error] 87-87: Forbidden non-null assertion. (lint/style/noNonNullAssertion)
Line range hint
1-1: Why are you importing types like they're going out of fashion? Useimport typeto make it clear that these are only for type checking and to help tree shaking do its job.- import { HttpErrorResponse } from '@angular/common/http'; + import type { HttpErrorResponse } from '@angular/common/http';Also applies to: 2-3, 3-4, 5-6, 6-7, 7-8, 8-9, 9-10, 10-11, 11-12, 13-14, 15-16
Tools
Biome
[error] 65-65: Forbidden non-null assertion. (lint/style/noNonNullAssertion)
[error] 87-87: Forbidden non-null assertion. (lint/style/noNonNullAssertion)
src/test/javascript/spec/component/assessment-dashboard/exam-assessment-buttons.component.spec.ts (2)
Line range hint
137-137: Non-null assertion in a test? Are you testing your luck or your code? Remove it and handle the potential null properly.- studentExamOne!.workingTime = 10; + if (!studentExamOne) throw new Error('studentExamOne is undefined'); + studentExamOne.workingTime = 10;
Line range hint
157-157: Here we go again. Non-null assertions in tests are as useful as a chocolate teapot. Fix it before it melts down your entire test suite.- jest.spyOn(examManagementService, 'evaluateQuizExercises').mockReturnValue(throwError(() => httpError)); + if (!httpError) throw new Error('httpError is undefined'); + jest.spyOn(examManagementService, 'evaluateQuizExercises').mockReturnValue(throwError(() => httpError));
...est/javascript/spec/component/assessment-dashboard/exam-assessment-buttons.component.spec.ts
Outdated
Show resolved
Hide resolved
iyannsch
dismissed stale reviews from coderabbitai[bot] and Strohgelaender
via
a61b350
iyannsch
requested review from
Strohgelaender,
florian-glombik,
Jan-Thurner and
Hialus
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range comments (2)
src/test/javascript/spec/component/assessment-dashboard/exam-assessment-buttons.component.spec.ts (2)
Line range hint
135-135: Non-null assertions again? Are you trying to summon a demon? This is TypeScript, not the Wild West. You can't just assert things will be non-null without consequences. Either handle the possibility of nulls gracefully or ensure your data integrity better upstream.- studentExamOne!.workingTime = 10; + if (!studentExamOne) throw new Error('Expected studentExamOne to be defined'); + studentExamOne.workingTime = 10; - jest.spyOn(examManagementService, 'assessUnsubmittedExamModelingAndTextParticipations').mockReturnValue(throwError(() => httpError)); + if (!examManagementService) throw new Error('Expected examManagementService to be defined'); + jest.spyOn(examManagementService, 'assessUnsubmittedExamModelingAndTextParticipations').mockReturnValue(throwError(() => httpError));Also applies to: 155-155
Line range hint
1-1: Oh, the horror! Importing types as regular imports? That's like using a chainsaw to cut a birthday cake. Useimport typeto keep your bundle size down and your transpiler happy. It's not rocket science, it's basic hygiene!- import { ComponentFixture, TestBed } from '@angular/core/testing'; + import type { ComponentFixture } from '@angular/core/testing'; + import { TestBed } from '@angular/core/testing'; - import { ActivatedRoute, Params, convertToParamMap } from '@angular/router'; + import type { Params } from '@angular/router'; + import { ActivatedRoute, convertToParamMap } from '@angular/router';Also applies to: 1-2
...est/javascript/spec/component/assessment-dashboard/exam-assessment-buttons.component.spec.ts
Show resolved
Hide resolved
|
@coderabbitai configuration |
CodeRabbit configurationlanguage: en-US
tone_instructions: ''
early_access: true
enable_free_tier: true
reviews:
profile: assertive
request_changes_workflow: true
high_level_summary: false
high_level_summary_placeholder: '@coderabbitai summary'
auto_title_placeholder: '@coderabbitai title'
poem: false
review_status: false
collapse_walkthrough: false
path_filters:
- '!dist/**'
- '!**/*.app'
- '!**/*.bin'
- '!**/*.bz2'
- '!**/*.class'
- '!**/*.db'
- '!**/*.csv'
- '!**/*.tsv'
- '!**/*.dat'
- '!**/*.dll'
- '!**/*.dylib'
- '!**/*.egg'
- '!**/*.glif'
- '!**/*.gz'
- '!**/*.xz'
- '!**/*.zip'
- '!**/*.7z'
- '!**/*.rar'
- '!**/*.zst'
- '!**/*.ico'
- '!**/*.jar'
- '!**/*.tar'
- '!**/*.war'
- '!**/*.lo'
- '!**/*.log'
- '!**/*.mp3'
- '!**/*.wav'
- '!**/*.wma'
- '!**/*.mp4'
- '!**/*.avi'
- '!**/*.mkv'
- '!**/*.wmv'
- '!**/*.m4a'
- '!**/*.m4v'
- '!**/*.3gp'
- '!**/*.3g2'
- '!**/*.rm'
- '!**/*.mov'
- '!**/*.flv'
- '!**/*.iso'
- '!**/*.swf'
- '!**/*.flac'
- '!**/*.nar'
- '!**/*.o'
- '!**/*.ogg'
- '!**/*.otf'
- '!**/*.p'
- '!**/*.pdf'
- '!**/*.doc'
- '!**/*.docx'
- '!**/*.xls'
- '!**/*.xlsx'
- '!**/*.map'
- '!**/*.out'
- '!**/*.ppt'
- '!**/*.pptx'
- '!**/*.pkl'
- '!**/*.pickle'
- '!**/*.pyc'
- '!**/*.pyd'
- '!**/*.pyo'
- '!**/*.pub'
- '!**/*.pem'
- '!**/*.rkt'
- '!**/*.so'
- '!**/*.ss'
- '!**/*.eot'
- '!**/*.exe'
- '!**/*.pb.go'
- '!**/*.pb.gw.go'
- '!**/*.lock'
- '!**/*.ttf'
- '!**/*.yaml'
- '!**/*.yml'
- '!**/*.xml'
- '!**/*.cfg'
- '!**/*.toml'
- '!**/*.ini'
- '!**/*.mod'
- '!**/*.sum'
- '!**/*.work'
- '!**/*.mmd'
- '!**/*.svg'
- '!**/*.jpeg'
- '!**/*.jpg'
- '!**/*.png'
- '!**/*.gif'
- '!**/*.bmp'
- '!**/*.tiff'
- '!**/*.webm'
- '!**/*.woff'
- '!**/*.woff2'
- '!**/*.dot'
- '!**/*.md5sum'
- '!**/*.wasm'
- '!**/*.snap'
- '!**/*.parquet'
- '!**/gen/**'
- '!**/_gen/**'
- '!**/generated/**'
- '!**/@generated/**'
- '!**/vendor/**'
- '!**/*.min.js'
- '!**/*.min.js.map'
- '!**/*.min.js.css'
- '!**/*.tfstate'
- '!**/*.tfstate.backup'
path_instructions:
- path: src/main/webapp/**/*.ts
instructions: >-
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
- path: src/main/java/**/*.java
instructions: >-
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
- path: src/test/javascript/spec/**/*.ts
instructions: >-
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}}
- path: src/test/java/**/*.java
instructions: >-
test_naming: descriptive; test_size: small_specific; fixed_data: true;
junit5_features: true; assert_use: assertThat; assert_specificity: true;
archunit_use: enforce_package_rules; db_query_count_tests:
track_performance; util_service_factory_pattern: true; avoid_db_access:
true; mock_strategy: static_mocks; context_restart_minimize: true
- path: src/main/webapp/**/*.html
instructions: >-
@if and @for are new and valid Angular syntax replacing *ngIf and
*ngFor. They should always be used over the old style.
- path: src/main/webapp/i18n/de/**/*.json
instructions: >-
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".
auto_review:
enabled: true
auto_incremental_review: true
ignore_title_keywords: []
labels: []
drafts: false
base_branches: []
tools:
shellcheck:
enabled: true
ruff:
enabled: true
markdownlint:
enabled: true
github-checks:
enabled: true
timeout_ms: 90000
languagetool:
enabled: true
disabled_rules:
- EN_UNPAIRED_BRACKETS
disabled_categories:
- TYPOS
- TYPOGRAPHY
- CASING
enabled_only: false
level: picky
enabled_rules: []
enabled_categories: []
biome:
enabled: true
hadolint:
enabled: true
swiftlint:
enabled: true
phpstan:
enabled: true
level: '5'
golangci-lint:
enabled: true
ast-grep:
packages: []
rule_dirs: []
util_dirs: []
essential_rules: true
chat:
auto_reply: true
knowledge_base:
learnings:
scope: auto
issues:
scope: auto
jira:
project_keys: []
linear:
team_keys: []
|
...dashboards/assessment-dashboard/exam-assessment-buttons/exam-assessment-buttons.component.ts
Show resolved
Hide resolved
...dashboards/assessment-dashboard/exam-assessment-buttons/exam-assessment-buttons.component.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Florian Glombik <[email protected]>
iyannsch
dismissed stale reviews from marlon-luca-bu, coderabbitai[bot], and Strohgelaender
via
e33399e
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range comments (3)
src/main/webapp/app/course/dashboards/assessment-dashboard/exam-assessment-buttons/exam-assessment-buttons.component.ts (3)
Line range hint
136-136: Specify an explicit type for this variable to avoid implicit 'any' type.let errorDetail: string | undefined;Tools
Biome
[error] 65-65: Forbidden non-null assertion. (lint/style/noNonNullAssertion)
[error] 87-87: Forbidden non-null assertion. (lint/style/noNonNullAssertion)
Line range hint
137-137: Use optional chaining to safely access nested properties.- if (err?.error && err.error.errorKey) { + if (err?.error?.errorKey) {Tools
Biome
[error] 65-65: Forbidden non-null assertion. (lint/style/noNonNullAssertion)
[error] 87-87: Forbidden non-null assertion. (lint/style/noNonNullAssertion)
Line range hint
1-1: Convert these imports to use 'import type' since they are only used as types. This will ensure they are removed by the transpiler, optimizing the module loading.import type { ActivatedRoute } from '@angular/router'; import type { Subscription, Observable } from 'rxjs'; import type { StudentExam } from 'app/entities/student-exam.model'; import type { CourseManagementService } from 'app/course/manage/course-management.service'; import type { Course } from 'app/entities/course.model'; import type { ExamManagementService } from 'app/exam/manage/exam-management.service'; import type { AlertService } from 'app/core/util/alert.service'; import type { HttpErrorResponse } from '@angular/common/http'; import type { Exam } from 'app/entities/exam.model'; import type { AccountService } from 'app/core/auth/account.service'; import type { ArtemisTranslatePipe } from 'app/shared/pipes/artemis-translate.pipe'; import type { faClipboard } from '@fortawesome/free-solid-svg-icons';Also applies to: 2-3, 3-4, 5-6, 6-7, 7-8, 8-9, 9-10, 10-11, 11-12, 13-14, 15-16
Tools
Biome
[error] 65-65: Forbidden non-null assertion. (lint/style/noNonNullAssertion)
[error] 87-87: Forbidden non-null assertion. (lint/style/noNonNullAssertion)
...dashboards/assessment-dashboard/exam-assessment-buttons/exam-assessment-buttons.component.ts
Show resolved
Hide resolved
...dashboards/assessment-dashboard/exam-assessment-buttons/exam-assessment-buttons.component.ts
Show resolved
Hide resolved
krusche
changed the title
Exam Mode: Utilize server endpoint for getting longest working time in exam assessmentDevelopment: Utilize server endpoint for getting longest working time in exam assessment
krusche
deleted the
chore/exam-mode/utilize-longest-working-time-server-endpoint
branch
Checklist
General
Server
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Changes affecting Programming Exercises
Motivation and Context
Fixes #8673
The server offers a new endpoint that allows to reduce the computational load on the client. This PR implements the changes in the client side.
Description
I removed getting all exams and calculating the longest working time iteratively by a simple call to the new endpoint.
Steps for Testing
Exam Mode Testing
Prerequisites:
Assessmenttab of the exam.Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Review Progress
Performance Review
Code Review
Manual Tests
Exam Mode Test
Test Coverage
Screenshots