Petryshyn/Create competitive event #2638
Conversation
DimaShapoval
force-pushed
the
Romanchak/Create-competitive-event
branch
from
901f6f9 to
0301665
Compare
WalkthroughThe changes introduce comprehensive competition management features throughout the application. New UI components have been added for displaying competition details, cards, and forms for creation and editing, with conditional rendering based on user roles. Additionally, the changes include new services, state actions, and selectors for competitions, alongside updates to models, constants, enums, and SCSS styling for improved consistency and responsiveness. Routing and module declarations have been updated to support the new competition-related flows while extensive test suites verify component and service functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CC as CreateCompetitionComponent
participant US as UserCompetitionService
participant ST as Store/State
U->>CC: Fill and submit competition form
CC->>US: Call createCompetition(competition)
US->>US: Decide API version and send HTTP request
US-->>CC: Return competition data
CC->>ST: Dispatch action to update state
ST-->>U: UI updates with new competition
Suggested reviewers
Poem
✨ Finishing Touches
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
CodeRabbit Configuration File (
|
DimaShapoval
force-pushed
the
Romanchak/Create-competitive-event
branch
3 times, most recently
from
02111dc to
a23cf28
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 30
🔭 Outside diff range comments (4)
src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.spec.ts (1)
1-61:🧩 Analysis chain
Verify test coverage metrics.
Let's check the actual test coverage for this component.
🏁 Script executed:
#!/bin/bash # Description: Check test coverage for ProviderCompetitionComponent # Run Jest with coverage for the specific component npx jest src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.spec.ts --coverageLength of output: 254
Jest Preset Configuration Issue for Test Coverage
The coverage command failed with "Preset jest-preset-angular not found." This suggests that the project’s Jest configuration is missing a necessary dependency, thus preventing proper coverage verification. Please manually verify that "jest-preset-angular" is installed and that your Jest configuration (typically in jest.config.js) correctly references it. This will be needed to generate accurate test coverage metrics.
Below is an additional shell script to help inspect the configuration:
After running the above, please review the output and adjust your setup accordingly to enable proper test coverage reporting.
🏁 Script executed:
#!/bin/bash echo "Checking for jest-preset-angular reference in package.json..." grep -R "jest-preset-angular" package.json || echo "No reference found in package.json." echo "Listing installed jest-preset-angular version..." npm ls jest-preset-angular || echo "jest-preset-angular is not installed."Length of output: 486
Jest Configuration Issue: Missing Dependency
The test coverage command fails because the package.json references "jest-preset-angular" (version "^14.0.3") but it isn't installed in node_modules. This missing dependency is causing the "Preset jest-preset-angular not found" error.
• Install jest-preset-angular (e.g., by running “npm install jest-preset-angular”)
• Verify that your Jest configuration (likely in jest.config.js) is set up correctly for Angular projectssrc/app/shell/details/side-menu/side-menu.component.ts (1)
37-46: Update getContactsData to handle competition input.The
getContactsDatamethod needs to be updated to handle the newly added competition input.Apply this change:
private getContactsData(): void { this.contactsData = { - phone: this.workshop?.phone || this.provider.phoneNumber, - email: this.workshop?.email || this.provider.email, - facebook: this.workshop?.facebook || this.provider.facebook, - instagram: this.workshop?.instagram || this.provider.instagram, - website: this.workshop?.website || this.provider.website + phone: this.competition?.phone || this.workshop?.phone || this.provider.phoneNumber, + email: this.competition?.email || this.workshop?.email || this.provider.email, + facebook: this.competition?.facebook || this.workshop?.facebook || this.provider.facebook, + instagram: this.competition?.instagram || this.workshop?.instagram || this.provider.instagram, + website: this.competition?.website || this.workshop?.website || this.provider.website }; - this.address = { ...(this.workshop?.address || this.provider?.actualAddress || this.provider.legalAddress) }; + this.address = { ...(this.competition?.address || this.workshop?.address || this.provider?.actualAddress || this.provider.legalAddress) }; }src/app/shared/components/validation-hint/validation-hint.component.ts (1)
156-161: Add null checks to prevent runtime errors.The
checkMatDatePickermethod assumes that the form control has 'start' and 'end' controls without checking their existence. This could lead to runtime errors.Add null checks to safely access the form controls:
private checkMatDatePicker(): void { this.invalidDateFormat = this.validationFormControl.hasError('matDatepickerParse'); this.invalidDateRange = this.validationFormControl.hasError('matDatepickerMin') || this.validationFormControl.hasError('matDatepickerMax'); - this.invalidStartEndDate = this.validationFormControl.get('start').valid && this.validationFormControl.get('end').invalid; + if (this.validationFormControl instanceof FormGroup) { + const startControl = this.validationFormControl.get('start'); + const endControl = this.validationFormControl.get('end'); + this.invalidStartEndDate = startControl?.valid && endControl?.invalid; + } }src/app/shared/components/sidenav-menu/sidenav-menu.component.html (1)
61-65: Suspicious Role Check for Administration Link
The condition at line 61 checks if
*ngIf="user.role === Role.provider && user.role === Role.providerDeputy"
which is logically impossible—a user cannot simultaneously have two different roles. Likely the intended operator is OR (||) rather than AND (&&). Consider changing the condition to ensure that users with either role see the administration link.Proposed change:
- *ngIf="user.role === Role.provider && user.role === Role.providerDeputy"
+ *ngIf="user.role === Role.provider || user.role === Role.providerDeputy"
🧹 Nitpick comments (53)
src/app/shell/personal-cabinet/personal-cabinet.component.html (1)
20-24: New Competitions Navigation Link Added CorrectlyThe new ng-container block for providers, which adds the competitions navigation link, is implemented consistently with the existing provider-specific links. The conditional check using isRoleProvider(userRole) and the use of translation and uppercase pipes ensure consistency with your UI conventions.
Suggestion: If more provider-specific links are added in the future, consider grouping them under a single ng-container to reduce repetitive role checks and improve maintainability.
src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.html (2)
1-5: Enhance Image Accessibility and Binding
The image element is correctly bound using Angular’s directives and leverages the conditional class assignment for responsive styling. For improved accessibility, consider dynamically constructing the alt text (e.g., using the judge’s full name) rather than a static "judge-photo".Proposed change:
- <img [ngClass]="{ 'card-img-full': judge.coverImageId }" mat-card-image [src]="coverImageUrl" alt="judge-photo" /> + <img [ngClass]="{ 'card-img-full': judge.coverImageId }" mat-card-image [src]="coverImageUrl" [alt]="judgeFullName || 'judge photo'" />
18-21: Judge Description Content Handling
The description is properly bound to display the judge’s information. Consider adding logic (or a fallback) to handle edge cases where the description might be empty or overly long, so that the UI remains consistent and user-friendly.src/app/shell/personal-cabinet/provider/create-competition/create-judge/judge-form/judge-form.component.html (1)
1-10: Consistent FormGroup Naming Recommendation
The form is bound with [formGroup]="JudgeForm", yet the validations are accessed via JudgeFormGroup (e.g., JudgeFormGroup.get('isChiefJudge')). Please verify that both refer to the same FormGroup instance; if so, consider using one consistent name to improve code readability and maintainability.src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.spec.ts (1)
17-26: Enhance mock coverage for better test scenarios.The current mocks could be expanded to cover more scenarios:
- Store mock could return different entity states for different test cases
- MatDialog mock could include scenarios where the user cancels the dialog
- Error scenarios should be included in the mocks
Consider updating the mocks like this:
storeMock = { dispatch: jest.fn(), - select: jest.fn().mockReturnValue(of({ entities: [], totalAmount: 0 })) + select: jest.fn().mockImplementation((selector) => { + // Different return values based on selector + return of({ entities: [], totalAmount: 0 }); + }) }; matDialogMock = { open: jest.fn().mockReturnValue({ - afterClose: () => of(true) + afterClose: () => of(true), // Default success case + // Add more mock implementations for different scenarios: + // afterClose: () => of(false), // Cancel case + // afterClose: () => throwError(() => new Error('Dialog error')), // Error case }) };src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.ts (1)
120-120: Remove or replace the console statement.Consider removing the debug statement "console.log(judges);" to maintain a clean production-ready codebase, or replace it with a logging utility if necessary.
- console.log(judges); + // Remove or replace with a logging utility as needed🧰 Tools
🪛 GitHub Check: build_and_test
[warning] 120-120:
Unexpected console statementsrc/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.ts (2)
77-98: Consider using subscriptions instead of array forEach.At lines 79-84, you’re sorting through institutions with a forEach, but for asynchronous flows, it is safer and more conventional to use subscription-based methods (e.g., .subscribe(...)) or an RxJS operator like .map(...) + .takeUntil(...) to ensure proper handling of multiple emissions.
105-167: Reduce repetitive patterns in radio button initialization.The four methods (onDisabilityOptionCtrlInit, onSelectionOptionsCtrlInit, onBenefitsOptionsCtrlInit, onPriceControlInit) share a similar enable/disable pattern. Consider creating a helper function to streamline repeated logic and improve maintainability.
src/app/shared/models/codeficator.model.ts (1)
14-15: Document the purpose of the order property.While the district property is self-explanatory, the purpose of the order property is unclear. Consider adding JSDoc comments to explain its usage.
Add documentation:
+ /** District name for the codeficator entry */ district?: string; + /** Determines the display order of codeficator entries (if applicable) */ order?: number;src/app/shared/models/judge.model.ts (1)
10-10: Consider using an enum for gender.The
genderproperty should be typed using an enum to ensure type safety and prevent invalid values.export enum Gender { Male = 'MALE', Female = 'FEMALE', Other = 'OTHER' }Then update the property:
- gender: string; + gender: Gender;src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.ts (1)
26-28: Add error handling for missing environment variable.The component should handle cases where the storage URL is not defined in the environment.
private getCoverImageUrl(): void { + if (!environment.storageUrl) { + console.warn('Storage URL is not defined in environment'); + } this.coverImageUrl = this.judge.coverImageId ? environment.storageUrl + this.judge.coverImageId : 'assets/icons/judge.png'; }src/app/shell/details/details-tabs/competition-judges/competition-judges.component.spec.ts (1)
26-39: Add more test coverage for judge properties.The test case for judge input only verifies basic property assignment. Consider adding tests for:
- Edge cases (empty array, null values)
- Chief judge functionality
- Judge property validations
it('it should expect correct input judge', () => { const mockJudge: Judge[] = [ { firstName: 'Jack', lastName: 'Smith', dateOfBirth: '', gender: 'Male', isChiefJudge: false, coverImageId: '' } ]; component.judges = mockJudge; expect(component.judges).toEqual(mockJudge); + // Test empty array + component.judges = []; + expect(component.judges).toEqual([]); + + // Test chief judge functionality + const chiefJudge = { ...mockJudge[0], isChiefJudge: true }; + component.judges = [chiefJudge]; + expect(component.judges[0].isChiefJudge).toBeTruthy(); });src/app/shell/personal-cabinet/provider/create-competition/create-judge/judge-form/judge-form.component.ts (2)
27-27: Remove unnecessary constructor.The empty constructor can be safely removed as it serves no purpose.
- constructor() {}🧰 Tools
🪛 Biome (1.9.4)
[error] 27-27: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
33-48: Consider using RxJS takeUntil for subscription cleanup.The subscription to
isChiefJudgevalueChanges should be properly cleaned up when the component is destroyed.+ private destroy$ = new Subject<void>(); + public ngOnInit(): void { this.JudgeForm.get('isChiefJudge') - ?.valueChanges.pipe(debounceTime(this.defaultDebounceTime), filter(Boolean)) + ?.valueChanges.pipe( + takeUntil(this.destroy$), + debounceTime(this.defaultDebounceTime), + filter(Boolean) + ) .subscribe(() => { // ... existing code ... }); } + + public ngOnDestroy(): void { + this.destroy$.next(); + this.destroy$.complete(); + }src/app/shared/store/shared-user.actions.ts (1)
84-87: Remove unnecessary constructor.The empty constructor in ResetProviderCompetitionDetails can be safely removed.
export class ResetProviderCompetitionDetails { static readonly type = '[user] clear Provider And Competition Details'; - constructor() {} }🧰 Tools
🪛 Biome (1.9.4)
[error] 86-86: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
src/app/shared/components/competition-card/competition-card.component.spec.ts (2)
60-60: Avoid using 'as any' type assertions.Replace type assertions with proper typing to maintain type safety.
- const competition: CompetitionProviderViewCard = { amountOfPendingApplications: 0, unreadMessages: 0 } as any; + const competition: CompetitionProviderViewCard = { + amountOfPendingApplications: 0, + unreadMessages: 0, + id: '1', + title: 'Test Competition', + // ... add other required properties + };Also applies to: 67-67
55-70: Enhance test coverage.Consider adding tests for:
- Edge cases in competition data
- Error scenarios
- Component lifecycle methods
- UI element interactions
src/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.ts (3)
20-22: Reorder member declarations to follow Angular style guide.Move public member declarations before protected ones to follow Angular's style guide recommendations.
+ public addressFormGroup: FormGroup; + public searchFormGroup: FormGroup; + public noAddressFound = false; protected readonly ValidationConstants = ValidationConstants; - public addressFormGroup: FormGroup; - public searchFormGroup: FormGroup; - public noAddressFound = false;🧰 Tools
🪛 GitHub Check: build_and_test
[warning] 20-20:
Member addressFormGroup should be declared before all protected instance field definitions
[warning] 21-21:
Member searchFormGroup should be declared before all protected instance field definitions
[warning] 22-22:
Member noAddressFound should be declared before all protected instance field definitions
34-49: Add type safety to form controls.The form controls lack explicit typing which could lead to runtime errors. Consider using strongly typed forms.
- public addressFormGroup: FormGroup; + public addressFormGroup: FormGroup<{ + street: FormControl<string>; + buildingNumber: FormControl<string>; + venue: FormControl<string>; + catottgId: FormControl<string>; + latitude: FormControl<string>; + longitude: FormControl<string>; + }>;
51-69: Enhance error handling in onAddressSelect.The error handling for geocoding could be more informative. Consider:
- Adding specific error messages for different failure scenarios
- Logging the error for debugging
public onAddressSelect(result: Geocoder): void { this.noAddressFound = !result; if (result) { this.addressFormGroup.patchValue( { latitude: result.lat, longitude: result.lon }, { emitEvent: false } ); if (result.codeficator) { this.settlementFormControl.setValue(result.codeficator, { emitEvent: false }); this.settlementSearchFormControl.setValue(result.codeficator.settlement, { emitEvent: false }); this.markFormAsDirtyOnUserInteraction(); } } else { - this.addressFormGroup.setErrors({ noAddressFound: true }); + this.addressFormGroup.setErrors({ + noAddressFound: true, + errorDetails: 'Failed to geocode the provided address' + }); + console.error('Geocoding failed for the provided address'); } }src/app/shared/components/competition-card/competition-card.component.ts (1)
43-43: Add type safety to destroy$ subject.The destroy$ subject should be explicitly typed as
voidsince it's only used for cleanup.- public destroy$: Subject<boolean> = new Subject<boolean>(); + public destroy$: Subject<void> = new Subject<void>();src/app/shared/constants/validation.ts (1)
75-79: Consider adding pattern validation for venue names.The venue validators could benefit from a pattern validation to ensure consistent formatting and prevent special characters.
+// Add to regex-constants.ts +export const VENUE_REGEX = /^[a-zA-Z0-9\s\-',.]+$/; static readonly defaultVenueValidators: ValidatorFn[] = [ Validators.required, Validators.minLength(ValidationConstants.INPUT_LENGTH_1), - Validators.maxLength(ValidationConstants.INPUT_LENGTH_60) + Validators.maxLength(ValidationConstants.INPUT_LENGTH_60), + Validators.pattern(VENUE_REGEX) ];src/app/shell/details/details.component.ts (2)
92-94: Update method documentation to include Competition.The method documentation only mentions Workshop and Provider, but the method now also handles Competition entities.
/** - * This method get Workshop or Provider by Id; + * This method gets Workshop, Competition, or Provider by Id; */
40-41: Initialize the competition property.For type safety, initialize the competition property to null or undefined.
- public competition: Competition; + public competition: Competition | null = null;src/app/shell/details/competition-details/competition-details.component.ts (1)
79-85: Remove commented-out code.Instead of keeping commented-out code for a future feature, consider creating a TODO task or issue to track this requirement.
Would you like me to help create an issue to track the publish competition feature implementation?
src/app/shell/details/competition-details/competition-details.component.spec.ts (1)
29-61: Merge duplicate beforeEach blocks.The static analysis tool correctly identified duplicate beforeEach hooks. These should be merged for better maintainability.
beforeEach(async () => { await TestBed.configureTestingModule({ imports: [ MatChipsModule, MatTabsModule, RouterTestingModule, MatIconModule, MatChipsModule, NgxsModule.forRoot([]), TranslateModule.forRoot(), BrowserAnimationsModule, MatDialogModule ], declarations: [ CompetitionDetailsComponent, MockCompetitionJudgesComponent, MockCompetitionAboutComponent, ImageCarouselComponent, MockActionsComponent, ConfirmationModalWindowComponent ], schemas: [NO_ERRORS_SCHEMA] }).compileComponents(); + fixture = TestBed.createComponent(CompetitionDetailsComponent); + component = fixture.componentInstance; + component.competition = {} as Competition; + component.provider = {} as Provider; + matDialog = TestBed.inject(MatDialog); + fixture.detectChanges(); }); - - beforeEach(() => { - fixture = TestBed.createComponent(CompetitionDetailsComponent); - component = fixture.componentInstance; - component.competition = {} as Competition; - component.provider = {} as Provider; - matDialog = TestBed.inject(MatDialog); - fixture.detectChanges(); - });🧰 Tools
🪛 Biome (1.9.4)
[error] 54-61: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.ts (2)
100-102: Remove commented-out code.Instead of keeping commented-out code for employee workshops, create a separate issue to track this feature implementation.
Would you like me to help create an issue to track the employee workshops feature implementation?
70-76: Use constant for dialog configuration.The dialog configuration object should be extracted to a constant to maintain consistency and avoid magic strings.
+ private readonly DIALOG_CONFIG = { + width: Constants.MODAL_SMALL, + data: { + type: ModalConfirmationType.delete + } + }; public onDelete(competition: CompetitionProviderViewCard): void { - const dialogRef = this.matDialog.open(ConfirmationModalWindowComponent, { - width: Constants.MODAL_SMALL, - data: { - type: ModalConfirmationType.delete, - property: competition.title - } - }); + const dialogRef = this.matDialog.open(ConfirmationModalWindowComponent, { + ...this.DIALOG_CONFIG, + data: { + ...this.DIALOG_CONFIG.data, + property: competition.title + } + });src/app/shared/services/competitions/user-competition.service.ts (2)
26-28: Consider adding error handling for invalid competition IDs.The
getCompetitionByIdmethod should handle potential 404 errors when an invalid ID is provided.public getCompetitionById(id: string): Observable<Competition> { - return this.http.get<Competition>(`/api/v1/CompetitiveEvent/${id}`); + return this.http.get<Competition>(`/api/v1/CompetitiveEvent/${id}`).pipe( + catchError(error => { + if (error.status === 404) { + return throwError(() => new Error('Competition not found')); + } + return throwError(() => error); + }) + ); }
33-45: Add pagination type safety.The
getProviderViewCompetitionsmethod should use type-safe pagination parameters.+interface PaginationParams { + from: number; + size: number; +} public getProviderViewCompetitions( - competitionCardParameters: CompetitionCardParameters + competitionCardParameters: CompetitionCardParameters & PaginationParams ): Observable<SearchResponse<CompetitionProviderViewCard[]>> {src/app/shell/personal-cabinet/provider/provider.module.ts (1)
63-73: Consider grouping related declarations.Competition-related components are scattered throughout the declarations array. Consider grouping them together for better organization.
declarations: [ // ... other components - CreateCompetitionComponent, - CreateRequiredFormComponent, CreateAdditionalAboutFormComponent, ProviderPositionsComponent, CreatePositionComponent, CreatePositionFormComponent, + // Competition related components + CreateCompetitionComponent, + CreateRequiredFormComponent, CreateCompetitionDescriptionFormComponent, CreateCompetitionAddressComponent, CreateJudgeComponent, JudgeFormComponent, ProviderCompetitionComponent ],src/app/shell/personal-cabinet/provider/create-competition/create-judge/create-judge.component.ts (1)
95-95: Remove redundant call tocheckedCountOfJudges.The
checkedCountOfJudgesmethod is already called within the dialog subscription and else block.- this.checkedCountOfJudges(); this.markFormAsDirtyOnUserInteraction();src/app/shared/store/shared-user.state.ts (1)
109-119: Consider using a specific error message for competition not found.The error handler uses a generic error message (
SnackbarText.error). Consider using a more specific message like other handlers (e.g.,SnackbarText.deletedWorkshopfor workshops).Apply this diff to improve error handling:
- dispatch(new ShowMessageBar({ message: SnackbarText.error, type: 'error' })); + dispatch(new ShowMessageBar({ message: SnackbarText.deletedCompetition, type: 'error' }));Also applies to: 147-151
🧰 Tools
🪛 Biome (1.9.4)
[error] 113-113: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.ts (2)
49-50: Consider using a date utility library.Direct date manipulation can be error-prone. Consider using a library like
date-fnsormoment.jsfor more reliable date operations.Example using date-fns:
import { addMonths, addYears, subMonths } from 'date-fns'; protected minDate: Date = subMonths(new Date(), 1); protected maxDate: Date = addYears(new Date(), 1); // In activateEditMode this.minDate = subMonths(new Date(this.competition.scheduledStartTime), 1);Also applies to: 122-124
191-195: Optimize form control listeners.Multiple separate subscriptions to form controls could impact performance. Consider:
- Combining related form control changes using
combineLatest.- Using
distinctUntilChanged()to prevent unnecessary updates.- Debouncing value changes for performance.
Example implementation:
private initListeners(): void { const providerInfo$ = this.useProviderInfoCtrl.valueChanges.pipe( distinctUntilChanged(), takeUntil(this.destroy$) ); const availableSeats$ = this.availableSeatsRadioBtnControl.valueChanges.pipe( distinctUntilChanged(), takeUntil(this.destroy$) ); const numberOfSeats$ = this.RequiredFormGroup.controls.numberOfSeats.valueChanges.pipe( distinctUntilChanged(), debounceTime(300), takeUntil(this.destroy$) ); providerInfo$.subscribe(/* ... */); availableSeats$.subscribe(/* ... */); numberOfSeats$.subscribe(/* ... */); }Also applies to: 204-213, 221-228, 241-245
src/app/shared/store/provider.state.ts (1)
974-984: Replace void with undefined in union types.Using
voidin union types can be confusing. Consider usingundefinedinstead for better type clarity.Apply this diff to improve type definitions:
- ): Observable<Competition | void> { + ): Observable<Competition | undefined> { - ): Observable<Competition | void> { + ): Observable<Competition | undefined> { - ): Observable<Competition | void> { + ): Observable<Competition | undefined> {Also applies to: 1006-1015, 1032-1041
🧰 Tools
🪛 Biome (1.9.4)
[error] 978-978: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/app/shell/details/details-tabs/competition-judges/competition-judges.component.html (1)
1-3: Good structure for conditional rendering and iterating judges.
The usage of *ngIf with optional chaining (judges?.length) and *ngFor is clear and concise. For enhanced performance when the judges list is large, consider implementing a trackBy function on the *ngFor directive.src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.scss (2)
12-14: Usage of ::ng-deep for targeting dropdown-panel.
While applying a max-height with !important is effective here, note that ::ng-deep is deprecated. Consider migrating to alternative strategies (such as global styles or using Angular’s ViewEncapsulation.None) in future updates.
22-28: Well-defined pseudo-element for seat decoration.
The .seat::after block provides a consistent styling cue. You might consider replacing the hard-coded color (#aaaaaa) with a SCSS variable for easier theming in the future.src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.html (2)
1-6: Clear “Add Competition” call-to-action section.
The section provides a translated prompt and a button styled as a raised button. For accessibility, consider adding an aria-label to the button.
8-17: Structured display of competition cards.
The use of an ng-container to conditionally render the competition cards ensures that content is shown only when not loading and when competitions exist. Integrating a trackBy in the *ngFor can further optimize performance.src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.scss (1)
22-24: Controlled dropdown-panel height using ::ng-deep.
As with similar styles, using ::ng-deep works well here, though exploring alternatives for future-proofing might be beneficial.src/app/shell/details/details-tabs/competition-about/competition-about.component.html (2)
1-16: Competition About Structure: The overall layout effectively displays competitive event description items, price information, and support details. One point to note—the title uses the translation key 'TITLES.ABOUT_THE_WORKSHOP'. If this view is dedicated to competitions, consider updating the key (or its value) to more accurately reflect the competition context.
17-33: Commented Lessons Schedule Section: The lessons schedule block is currently commented out. It would improve clarity to either remove this code if it’s no longer needed or add a comment explaining its intended future use.src/app/shared/components/competition-card/competition-card.component.scss (1)
1-147: SCSS for Competition Card Component:
The stylesheet is well structured with clear use of imports, flex layouts, and media queries for responsiveness. One recommendation is to avoid or reduce reliance on ::ng-deep if possible since its long-term support in Angular is uncertain; consider alternative encapsulation strategies. Overall, the styles effectively enhance the component’s appearance and usability.src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.html (2)
4-12: Header Section and Translations
The header block (lines 4–12) correctly uses translation pipes for dynamic titles and descriptions. Ensure that the translation keys such as 'FORMS.HEADERS.EDIT_COMPETITION' and 'FORMS.HEADERS.NEW_COMPETITION' match across the project for consistency.
61-78: Address Form Step and Navigation Flow
The third step uses AddressFormGroup with a completion condition that depends on both RequiredFormGroup and DescriptionFormGroup. The integration of with form passing and validatory navigation is correct. Consider adding ARIA labels to the buttons for improved accessibility.src/app/shared/components/sidenav-menu/sidenav-menu.component.html (1)
1-7: General Structure and TODO Reminder
The file starts with a TODO comment about making links correspondent with the header component. Ensure that either this TODO is eventually addressed or an issue is tracked for consistency in navigation element styles.src/app/shell/personal-cabinet/provider/create-workshop/create-workshop.component.html (1)
16-18: Step Label Update
At line 17, the step label key has been updated from a shortened version to 'TITLES.ABOUT_THE_WORKSHOP'. This enhances clarity even if it slightly increases verbosity in the UI.src/app/shared/components/competition-card/competition-card.component.html (2)
48-57: Provider Actions Template
The ProviderActions template renders edit and delete buttons and uses routerLink to navigate to the competition editing page. Consider adding a confirmation prompt before executing onDelete() if one isn’t already implemented in the component’s TypeScript file.
94-158: User Workshop View Rendering
The UserWorkshopView template displays key competition details such as rating, planned format, age ranges, and pricing. The commented-out block regarding the organizer’s providerTitle is acceptable for now but consider removing or marking it with a TODO if it’s not needed long term.src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.html (2)
10-12: Typo in Variable Name for Category List
The ngFor loop iterates over “instituitionsHierarchy$”. This appears to be a potential typo; consider renaming it to “institutionsHierarchy$” for clarity and consistency.
229-258: Competitive Benefits Configuration
The competitive benefits segment mirrors the structure of other sections with a radio group and descriptive textarea.
• Note a minor potential typo: the radio button name attribute is set as "benefitsnRadioBtn2" (line ~232). Renaming it to "benefitsRadioBtn2" would improve clarity and consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/assets/i18n/en.jsonis excluded by!src/assets/**and included bysrc/**src/assets/i18n/uk.jsonis excluded by!src/assets/**and included bysrc/**
📒 Files selected for processing (83)
src/app/header/header.component.html(1 hunks)src/app/shared/components/competition-card/competition-card.component.html(1 hunks)src/app/shared/components/competition-card/competition-card.component.scss(1 hunks)src/app/shared/components/competition-card/competition-card.component.spec.ts(1 hunks)src/app/shared/components/competition-card/competition-card.component.ts(1 hunks)src/app/shared/components/sidenav-menu/sidenav-menu.component.html(1 hunks)src/app/shared/components/validation-hint/validation-hint.component.html(2 hunks)src/app/shared/components/validation-hint/validation-hint.component.ts(2 hunks)src/app/shared/components/workshop-card/workshop-card.component.ts(1 hunks)src/app/shared/constants/constants.ts(3 hunks)src/app/shared/constants/validation.ts(1 hunks)src/app/shared/enum/competition.ts(1 hunks)src/app/shared/enum/enumUA/competition.ts(1 hunks)src/app/shared/enum/enumUA/message-bar.ts(1 hunks)src/app/shared/enum/enumUA/navigation-bar.ts(1 hunks)src/app/shared/enum/enumUA/no-results.ts(1 hunks)src/app/shared/enum/modal-confirmation.ts(6 hunks)src/app/shared/models/address.model.ts(2 hunks)src/app/shared/models/codeficator.model.ts(1 hunks)src/app/shared/models/competition.model.ts(1 hunks)src/app/shared/models/institution.model.ts(1 hunks)src/app/shared/models/judge.model.ts(1 hunks)src/app/shared/models/provider.model.ts(1 hunks)src/app/shared/services/competitions/user-competition.service.spec.ts(1 hunks)src/app/shared/services/competitions/user-competition.service.ts(1 hunks)src/app/shared/services/images/images.service.ts(2 hunks)src/app/shared/shared.module.ts(3 hunks)src/app/shared/store/provider.actions.ts(3 hunks)src/app/shared/store/provider.state.ts(7 hunks)src/app/shared/store/shared-user.actions.ts(3 hunks)src/app/shared/store/shared-user.state.ts(7 hunks)src/app/shared/styles/create-form.scss(1 hunks)src/app/shared/utils/utils.ts(2 hunks)src/app/shell/details/competition-details/competition-details.component.html(1 hunks)src/app/shell/details/competition-details/competition-details.component.scss(1 hunks)src/app/shell/details/competition-details/competition-details.component.spec.ts(1 hunks)src/app/shell/details/competition-details/competition-details.component.ts(1 hunks)src/app/shell/details/details-tabs/competition-about/competition-about.component.html(1 hunks)src/app/shell/details/details-tabs/competition-about/competition-about.component.scss(1 hunks)src/app/shell/details/details-tabs/competition-about/competition-about.component.spec.ts(1 hunks)src/app/shell/details/details-tabs/competition-about/competition-about.component.ts(1 hunks)src/app/shell/details/details-tabs/competition-judges/competition-judges.component.html(1 hunks)src/app/shell/details/details-tabs/competition-judges/competition-judges.component.scss(1 hunks)src/app/shell/details/details-tabs/competition-judges/competition-judges.component.spec.ts(1 hunks)src/app/shell/details/details-tabs/competition-judges/competition-judges.component.ts(1 hunks)src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.html(1 hunks)src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.scss(1 hunks)src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.spec.ts(1 hunks)src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.ts(1 hunks)src/app/shell/details/details.component.html(2 hunks)src/app/shell/details/details.component.ts(5 hunks)src/app/shell/details/details.module.ts(2 hunks)src/app/shell/details/side-menu/side-menu.component.ts(2 hunks)src/app/shell/personal-cabinet/personal-cabinet.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.scss(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.scss(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.scss(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-judge/create-judge.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-judge/create-judge.component.scss(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-judge/create-judge.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-judge/judge-form/judge-form.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-judge/judge-form/judge-form.component.scss(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-judge/judge-form/judge-form.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.scss(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-workshop/create-about-form/create-about-form.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-workshop/create-workshop.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-workshop/create-workshop.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.html(1 hunks)src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.scss(1 hunks)src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.spec.ts(1 hunks)src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/provider-routing.module.ts(1 hunks)src/app/shell/personal-cabinet/provider/provider.module.ts(2 hunks)src/app/shell/personal-cabinet/shared-cabinet/applications/application-card/application-card.component.html(2 hunks)src/app/shell/shell-routing.module.ts(2 hunks)
✅ Files skipped from review due to trivial changes (12)
- src/app/shell/details/details-tabs/competition-judges/competition-judges.component.scss
- src/app/shell/personal-cabinet/provider/create-competition/create-judge/judge-form/judge-form.component.scss
- src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.scss
- src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.scss
- src/app/shell/personal-cabinet/provider/create-competition/create-judge/create-judge.component.scss
- src/app/shell/personal-cabinet/provider/create-workshop/create-about-form/create-about-form.component.ts
- src/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.scss
- src/app/shell/personal-cabinet/provider/create-competition/create-judge/create-judge.component.html
- src/app/shell/details/competition-details/competition-details.component.scss
- src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.scss
- src/app/shell/details/details-tabs/competition-about/competition-about.component.scss
- src/app/shared/components/workshop-card/workshop-card.component.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/app/shared/store/shared-user.actions.ts
[error] 86-86: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
src/app/shared/store/shared-user.state.ts
[error] 113-113: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/app/shell/personal-cabinet/provider/create-competition/create-judge/judge-form/judge-form.component.ts
[error] 27-27: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
src/app/shell/details/competition-details/competition-details.component.spec.ts
[error] 54-61: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
src/app/shared/store/provider.state.ts
[error] 978-978: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 1010-1010: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 1036-1036: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🪛 GitHub Check: build_and_test
src/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.ts
[warning] 20-20:
Member addressFormGroup should be declared before all protected instance field definitions
[warning] 21-21:
Member searchFormGroup should be declared before all protected instance field definitions
[warning] 22-22:
Member noAddressFound should be declared before all protected instance field definitions
src/app/shell/personal-cabinet/provider/provider.module.ts
[warning] 37-37:
This line has a length of 163. Maximum allowed is 140
src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.ts
[warning] 120-120:
Unexpected console statement
🪛 GitHub Actions: Lint, build and test
src/app/shell/details/competition-details/competition-details.component.ts
[error] 65-65: Property 'setCarouselImages' does not exist on type 'ImagesService'. Did you mean 'getCarouselImages'?
🔇 Additional comments (100)
src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.html (1)
7-17: Validate Tooltip and Header Rendering
The header section efficiently presents the judge’s full name along with a tooltip. Please verify that the custom directive or property "showTooltipIfTruncated" is implemented and functions as intended alongside Angular Material’s tooltip functionality. Also, ensure that the bound "tooltipPosition" meets the UI/UX expectations.src/app/shell/personal-cabinet/provider/create-competition/create-judge/judge-form/judge-form.component.html (10)
12-27: Last Name Field Configuration
The last name field is well configured with a proper label, input element, and appropriate validation hint parameters (min/max characters). The use of appTrimValue to sanitize input is a good practice.
29-44: First Name Field Setup
The first name input field follows the same established pattern as the last name field. The label, placeholder, focusout event, and validation settings seem correctly implemented.
46-52: Gender Selection via Radio Buttons
The gender selection is implemented using a mat-radio-group with two clearly defined radio buttons. The integration with the translation pipe for label text is properly applied. Consider verifying that any ARIA attributes needed for enhanced accessibility are covered by Angular Material defaults.
53-67: Date of Birth Input and Datepicker Integration
The date picker input is correctly bounded by [min] and [max] (using values like today and minDate) and bound with a mat-datepicker. Additionally, the associated validation hint uses the [minMaxDate] directive. Ensure that the component correctly initializes and updates these date boundary properties.
69-73: Judge Information Label with Character Count Display
The section providing the judge’s description label includes a live character count, which is a helpful user feedback feature. Verify that JudgeFormGroup.get('description').value is always defined or handled gracefully (using the safe navigation ".value?") to avoid runtime errors if the value is null.
75-87: Description Textarea Field Setup
The textarea for judge information is implemented with clear placeholder text, maxlength restriction, and appTrimValue. The setup supports Angular’s reactive validation effectively. Ensure that any directive-specific behavior (such as appTrimValue) works consistently across both input and textarea elements.
88-92: Validation Hint for Description Field
The validation component for the description field properly receives boundaries via minCharacters and maxCharacters inputs, maintaining consistency with validationConstants. Confirm that these constants match the business rules for description lengths.
94-98: Conditional Display of Delete Judge Button
The delete judge button is conditionally rendered when index !== 0, providing a user-friendly way to remove multiple judges. Verify that the onDeleteJudge() method exists in the component and handles removal logic correctly.
100-100: Conditional Separator Element
The conditional divider (border-bottom) is displayed based on judgeAmount and index to visually separate entries when multiple judges are present. The condition appears logical and supports better UI clarity.
101-102: Proper Form Closure
The form element is appropriately closed, ensuring that the reactive form binding is sustained throughout.src/app/shared/enum/competition.ts (3)
1-6: LGTM! Well-structured status progression.The enum provides a clear and logical progression of competition states from Draft to Archived.
15-22: LGTM! Comprehensive coverage levels.The enum provides a well-structured hierarchy of geographical coverage levels from Local to International.
24-28: LGTM! Clear learning mode categorization.The enum effectively captures the three main modes of learning with consistent naming.
src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.spec.ts (1)
1-9: LGTM! Imports are well-organized and complete.All necessary testing dependencies are properly imported.
src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.ts (4)
63-72: Ensure inherited methods are called in the correct order.You are calling determineEditMode(), determineRelease(), and addNavPath() without showing their implementations here. Make sure these inherited or parent-class methods are invoked in a sensible order to avoid inconsistent state initialization.
100-109: Efficient subscription management.The setEditMode method correctly dispatches GetCompetitionById and subscribes to the competition observable. Ensure that no race conditions occur if the store updates the competition multiple times, which could override local changes unintentionally.
114-131: Overall form submission flow looks good.The onSubmit() logic for creating or updating competitions is clear and consistent, correctly dispatching new store actions based on editMode. Good job enforcing typed structures for competition objects.
🧰 Tools
🪛 GitHub Check: build_and_test
[warning] 120-120:
Unexpected console statement
173-176: Proper cleanup in ngOnDestroy.Dispatching ResetProviderCompetitionDetails during component teardown is good practice to maintain a clean store state.
src/app/shared/models/competition.model.ts (2)
58-69: Constructor logic is consistent.The constructor initializes fundamental fields with required and description data. The usage of provider.id for organizerOfTheEventId is straightforward and clear.
139-159: Extended constructor usage is clear.The derived class correctly delegates to the base constructor and then sets additional fields like coverImageId. This aligns well with an inheritance pattern.
src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.ts (1)
182-204: Verify benefitsOptionRadioBtn usage.When areThereBenefits is true, you set benefitsOptionRadioBtn to this.competition.benefits (line 201), while the property name in the model is areThereBenefits. Ensure that applying .setValue(...) is done with the correct source property since the code references “benefits” instead of “areThereBenefits.”
src/app/shell/details/details-tabs/competition-about/competition-about.component.ts (1)
1-12: LGTM! Well-structured Angular component.The component follows Angular's best practices with proper decorators, clear input property typing, and clean organization.
src/app/shared/models/address.model.ts (1)
7-7: LGTM!The addition of the optional
venueproperty and its initialization in the constructor is well-implemented and maintains backward compatibility.Also applies to: 16-16
src/app/shared/enum/enumUA/no-results.ts (1)
13-15: LGTM!The new enum entries follow the established pattern and enhance the coverage for competition-related scenarios.
src/app/shared/models/institution.model.ts (1)
8-8: LGTM!The addition of the optional
isGovernmentproperty follows TypeScript best practices and maintains backward compatibility.src/app/shell/personal-cabinet/provider/provider-routing.module.ts (1)
9-9: LGTM!The new route for competitions is correctly configured and follows the established pattern in the module.
Also applies to: 17-18
src/app/shell/details/details.module.ts (1)
20-23: LGTM! Components are properly organized.The new competition-related components are correctly imported and declared in the module.
Also applies to: 40-43
src/app/shared/components/competition-card/competition-card.component.ts (1)
50-61: I'll run a couple of shell script commands to confirm that the updated code includes the new TODO comment and guard clause. This will help verify that the stub code has been properly addressed.src/app/shared/enum/enumUA/navigation-bar.ts (1)
47-50: LGTM! Navigation bar entries for competitions added correctly.The new competition-related entries follow the existing naming pattern and translation key structure.
src/app/shared/constants/constants.ts (3)
17-18: LGTM! Good refactoring of constant names.Removing the "WORKSHOP" prefix makes these constants more reusable across different features.
79-79: LGTM! Consistent pagination constant.The
COMPETITIONS_PER_PAGEvalue aligns with other similar constants.
155-155: LGTM! Appropriate mode constant addition.The
COMPETITIONmode constant follows the existing pattern.src/app/shared/enum/enumUA/message-bar.ts (1)
17-19: LGTM! Well-structured message additions.The new competition-related messages follow the established naming pattern and are appropriately grouped.
src/app/shared/models/provider.model.ts (1)
186-186: LGTM!The new property
excludedCompetitionIdfollows the same pattern as the existingexcludedWorkshopIdproperty, maintaining consistency in the interface.src/app/shell/personal-cabinet/provider/create-workshop/create-workshop.component.ts (1)
52-52: LGTM!The constant reference has been updated to use the consolidated
UNLIMITED_SEATSconstant, maintaining consistency across the application.src/app/shared/store/shared-user.state.ts (2)
11-11: LGTM!The state model changes and imports are well-structured and align with the competition feature implementation.
Also applies to: 17-17, 45-45, 56-56
90-93: LGTM!The selector implementation is consistent with the existing pattern.
src/app/shell/shell-routing.module.ts (1)
181-194: LGTM!The new routes for create-competition follow the established pattern and include appropriate guards.
src/app/shared/enum/modal-confirmation.ts (1)
5-5: LGTM!The new enum values are well-organized and follow the existing pattern.
Also applies to: 47-49, 69-69, 102-104, 118-118, 151-153
src/app/shared/store/provider.actions.ts (1)
482-528: LGTM!The competition-related actions are well-structured and follow NGRX best practices. The actions cover all necessary CRUD operations with proper success and failure handling.
src/app/shell/details/details-tabs/competition-judges/competition-judges.component.html (1)
4-6: Effective fallback display for an empty judges list.
Using an ng-template with a dedicated component (app-no-result-card) ensures a graceful UI when there are no judge entries.src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.scss (5)
1-6: Consistent use of shared style imports.
The imports from shared styles (create-form, validation-form, dropdown, toggle-button, and info-hint) help maintain consistency across components.
7-10: Clear styling for seat-input elements.
The .seat-input class with fixed max-width and centered text provides a straightforward and effective style for input elements in this context.
16-20: Responsive design for container elements.
The media query adjusts the .container to a column direction and resets alignment, which is good for smaller screens. Ensure that these changes are consistent with the overall responsive design strategy of the application.
30-32: Appropriate alignment for radio-button-wrap.
Using place-content with !important ensures the intended alignment, though make sure the use of !important is necessary and doesn’t conflict with other styles.
35-37: Ensuring form field infix widths adapt dynamically.
The :host ::ng-deep rule sets the .mat-form-field-infix width to auto, helping the input fields to adjust to content.src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.html (1)
18-26: Robust conditional pagination rendering.
Displaying the paginator only when competitions.totalAmount exists ties the feature closely to actual data. Verify that competitions.totalAmount is always defined when competitions are loaded.src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.scss (6)
1-6: Consistent shared style imports maintained.
The imported styles (create-form, validation-form, dropdown, toggle-button, info-hint) promote uniformity across similar components.
7-10: Base styling for .price-text is clean and effective.
Defining a fixed width and block display ensures that .price-text is rendered consistently across browsers.
12-15: Default hidden state for .price-only-uah and .for-only-text.
The choice to hide these elements by default is clear; they are later toggled in the media query for responsiveness.
17-20: .price-input styling offers clear constraints and alignment.
The max-width and center alignment facilitate a clean layout for price input fields.
26-57: Responsive adjustments via media queries.
Within the media query, the .container is set to a column layout, .price::after is styled for emphasis, and display properties for .price-text, .price-only-uah, and .for-only-text are toggled appropriately. The grouped changes for .price, .radio-button-wrap, and .step-input-age also ensure proper alignment and sizing on smaller screens. This consolidated approach supports a flexible design adaptation.
59-61: Adaptive form field width for improved layout.
The :host ::ng-deep rule ensures that the .mat-form-field-infix adapts its width, which is particularly useful in responsive designs.src/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.html (3)
1-5: Effective encapsulation of the address form section.
Wrapping the component inside a bordered div ensures visual separation from the rest of the page. The structure is clear and supports modularity.
6-16: Well-implemented reactive form for venue input.
The form binds to addressFormGroup and employs material input coupled with a custom directive (appTrimValue) to ensure clean input. The accompanying app-validation-hint effectively augments user feedback.
17-19: Clear feedback for invalid address selection.
Displaying a conditional warning message and prompt text supports usability by guiding the user when no address is found.src/app/shell/details/details.component.html (3)
2-2: Conditional Rendering Update: The ng-container now uses *ngIf with the then/else syntax to switch between Workshop and Competition details. Please confirm that the flags (isWorkshop and, indirectly, isCompetition) are managed so that only one branch is active as intended.
7-7: Input Property Addition: The binding [competition]="competition" has been added to the app-side-menu component. Ensure that the app-side-menu component’s TypeScript and template files are updated to accept and properly use this new input property.
26-35: New Competition Details Template: The newly introduced <ng-template #CompetitionDetails> block organizes the competition details rendering clearly. Verify that the conditions using isCompetition and competition correctly control the display, and that the fallback to ProviderDetails works as expected when competition data is missing.src/app/shared/styles/create-form.scss (1)
96-97: Updated Border Styling:
The modification to the .step-border class—adding a padding-bottom of 1rem and a border-bottom of 2px solid #e3e3e3—brings better visual separation to step sections. Please verify that this new style is consistent with the overall design system.src/app/shared/components/validation-hint/validation-hint.component.html (2)
11-11: Enhanced Validation Condition:
A new conditional check for invalidStartEndDate has been added. This improves the form’s feedback for erroneous date ranges. Ensure that the business logic in the component’s code validates this condition as expected.
57-58: New Validation Template Added:
The template for invalidStartEndDateTmpl follows the established pattern used in the other validation templates. Please ensure that the translation key 'FORMS.VALIDATIONS.INVALID_START_END_DATES' is defined and correctly conveys the error message to users.src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.html (4)
1-3: Initial Conditional Rendering and Container Structure
The use of the ng-container with an *ngIf="provider" is appropriate to ensure that content renders only when provider data is available. Also, the container div that checks (editMode && competition) || !editMode fits the conditional display logic.
14-35: Stepper for Basic Competition Details
The implementation of the mat-horizontal-stepper with its first step (lines 14–35) is well-structured. The use of [stepControl]="RequiredFormGroup" along with proper navigation buttons (with disabling logic based on RequiredFormGroup?.invalid) confirms that users cannot progress without valid input.
37-59: Description Form Step
The second step with DescriptionFormGroup is implemented correctly and relies on the prior step’s validity by using [completed]="!RequiredFormGroup.invalid". It efficiently integrates the app-create-competition-description-form component and proper navigation buttons (Back, Cancel, Next).
80-117: Judge Selection Step with Final Submission Conditions
The final step for judges selection uses JudgeFormArray as stepControl. The composite condition for enabling the "Save" button efficiently checks for loading state, form “dirtiness” (ensuring some data was modified), and validity of all steps. Double-check that the logic for checking if all form groups are dirty fits the intended UX; sometimes, you might want to allow submission even if one of the controls isn’t dirty if it has already been submitted before.src/app/shared/components/sidenav-menu/sidenav-menu.component.html (1)
50-56: Provider-Specific Navigation Link for Competitions
The new link inserted at lines 53–55, which conditionally renders for provider users, is added correctly. It uses the proper translation key 'ENUM.NAV_BAR_NAME.MY_COMPETITIONS' and a clear routerLink path. This improves navigation for provider roles.src/app/shell/details/competition-details/competition-details.component.html (4)
1-7: Overall Layout and Container Usage
The container div encapsulates the entire competition details view and includes an image carousel component. The structure is clear and separates action buttons from details effectively.
5-12: Action Buttons for Competition Management
The Archive, Edit, and Publish buttons added in lines 6–12 are intuitively placed. Verify that the publish button’s disable logic ([disabled]="competition.state !== competitionStatus.Draft") accurately reflects the business rules (i.e., publishing might only be allowed for competitions in the Draft state).
23-32: Competition Basic Data Display
The details section correctly presents competition attributes (title, short title, recruitment status chip, etc.) using conditional rendering. Verify that the ng-container switch between competitionStatusOpen and the ClosedWorkshopStatus template properly reflects the competition’s state.
112-122: Closed Competition Status Template
The ng-template for ClosedWorkshopStatus (lines 111–122) dynamically applies styles based on competition.state. Double-check that the conditions for 'chip-color-close' and 'chip-color-draft' cover all relevant states, and that the displayed translation accurately communicates the status to the user.src/app/shell/personal-cabinet/provider/create-workshop/create-workshop.component.html (2)
1-9: Translation Keys Update in the Header
The header of the workshop creation component reflects changes in translation keys (line 9 now uses 'FORMS.HEADERS.EDIT_DESCRIPTION_PATH' or 'FORMS.HEADERS.NEW_DESCRIPTION_PATH'). This change is consistent with efforts to simplify translation keys.
14-141: Stepper Flow and Form Integration
The multi-step form is laid out with clear separation of concerns: About, Additional About, Description, Address, and Teacher selection steps. The use of form group passing from child components (via events like onReceiveAboutFormGroup, onReceiveAdditionalAboutGroup, etc.) supports modularity. Ensure that the overall navigation and the handling of "dirty" status across the multiple form groups meet the desired UX.src/app/shell/personal-cabinet/shared-cabinet/applications/application-card/application-card.component.html (3)
31-39: Updated Available Seats Condition
The condition on line 33 checks if the workshop availableSeats property is not equal to Constants.UNLIMITED_SEATS. This change (from the old constant key) aligns with the renaming and ensures that available seats are only displayed when limited. Verify that Constants.UNLIMITED_SEATS is used consistently across the project.
39-47: Display of Provider and Organization Details
The remainder of the file displays workshop details, provider information, and organizational data with clear use of Angular directives. Ensure that subtraction for available seats (availableSeats minus takenSeats) handles edge cases (e.g., when takenSeats might accidentally exceed availableSeats).
89-98: Action Buttons for Application Management
The action buttons for parents (e.g., "Leave Workshop") in lines 90–98 are correctly conditionally rendered based on the application status. The encapsulation within a flex container simplifies layout management.src/app/shared/components/competition-card/competition-card.component.html (5)
1-2: Initial Conditional Rendering Check
Using *ngIf on the to ensure that competitionData exists is a good safeguard.
6-11: Image Display Section Verification
Thetag uses a conditional class based on competitionData.coverImageId and binds its src to competitionData['_meta']. Please verify that the _meta property reliably provides the expected image URL. Consider renaming this property to a more descriptive name if possible.
14-30: Header and Direction Icons Section
The component renders the competition title inside a mat-card-title, and iterates over competitionData.directionIds to display icons. The use of ng-container for conditional rendering is clear and effective.
32-43: Competition Details & Navigation Footer
The mat-card-content and mat-card-footer sections correctly conditionally render additional competition details and navigation links. Ensure that the “isCabinetView” and “isCreateFormView” conditions align with the intended user experience.
59-92: Provider Competition View Logic
In the ProviderCompetitionView template, the conditional check uses “competitionData.state === workshopStatus.Open” along with an availableSeats comparison. Since this feature is for competitions, please double-check that using “workshopStatus” here is intentional. If the domain model is being repurposed, a renaming for clarity might be beneficial.src/app/header/header.component.html (3)
1-3: Header Container Structure
The overall header container and nested divs use Angular’s structural directives effectively to manage layout and conditional styling.
23-39: Language Switcher Implementation
The language selection button leverages mat-select and appears to offer a good user experience for switching languages. No issues detected here.
118-178: User Menu and New Competitions Button
Within the userView template, the new block (lines 133-137) for providers now includes a “MY_COMPETITIONS” button that links to '/personal-cabinet/provider/competitions'. This addition aligns well with the competition management feature set.src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.html (5)
uses the reactive form group “RequiredFormGroup”, and the checkbox to toggle provider contact details is correctly implemented.
1-5: Form Container Initialization and Provider Info Toggle
The base
6-13: Image Upload Control Setup
The is conditionally rendered based on the isImagesFeature flag and is set up with necessary inputs such as [imgMaxAmount] and [cropperConfig]. This modular approach supports feature toggling effectively.
109-122: Form of Learning Selection
The usage of a mat-select for choosing the form of learning is straightforward and paired with corresponding validation hints.
124-155: Available Places Section and Radio Button Group
The available places section conditionally renders based on provider ownership and includes a radio group for selecting between unlimited seats and specifying a number.
• Notice that elements such as the container div and radio-button-wrap have duplicate 'class' attributes (e.g., line 133 and line 134). Please combine multiple classes into a single attribute to ensure that all classes are applied correctly.
• Additionally, placing an inside a is unconventional; ensure that the styling and accessibility meet your requirements.
156-157: Form Closure and Overall Structure
The form structure is complete and neatly organized, providing clear input fields and validation components throughout.src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.html (10)
1-4: Competition Description Form Initialization
The form's reactive FormGroup “DescriptionFormGroup” is properly bound, and the initial category selection field is set up with a clear label and a mat-select control.
18-27: Subcategory Input and Validation
The subcategory field is straightforward with an input control and a linked validation hint enforcing proper character limits.
28-33: Description Field with Character Count Display
Displaying a real-time character count next to the description label is a nice touch for improved user feedback.
53-69: Coverage Selection
The mat-select for choosing the competition coverage is properly set up and includes an accompanying validation hint.
72-79: Form of Learning via Radio Group
This section leverages a mat-radio-group to allow selection of a learning form, iterating over the available options. The layout is clear.
81-98: Disability Support Options
The radio group for support for persons with disabilities, along with the conditional input for additional details, is well structured.
100-118: Additional Description Input
The additional description textarea along with its real-time character count and validation hints are clearly implemented.
125-174: Age Range Controls for Participants
The dual input fields for defining the age range (minAge and maxAge) include validations that dynamically adjust based on the current input values. The binding of [min] and [max] is thoughtfully done to enforce proper age constraints.
177-206: Competitive Selection Section
This section uses a radio group to toggle the competitive selection option and conditionally enable a descriptive textarea. The layout includes a live character count display.
208-227: Price Input Group with Conditional Radio Button
The price section efficiently uses a radio group to toggle between free and paid options while providing a numeric input for pricing when applicable.
src/app/shared/enum/competition.ts
Outdated
| export enum TypeOfCompetition { | ||
| EducationalProject = 'EducationalProject', | ||
| Competition = 'Competition', | ||
| ParentCompetition = 'MainCompetition', | ||
| CompetitionStage = 'CompetitionStage' | ||
| } |
There was a problem hiding this comment.
Fix inconsistent naming in TypeOfCompetition enum.
The enum key ParentCompetition maps to value 'MainCompetition', which creates inconsistency. This might cause confusion in code that references this enum.
Choose one consistent naming convention:
export enum TypeOfCompetition {
EducationalProject = 'EducationalProject',
Competition = 'Competition',
- ParentCompetition = 'MainCompetition',
+ ParentCompetition = 'ParentCompetition', // Option 1: Match the key
+ MainCompetition = 'MainCompetition', // Option 2: Use Main consistently
CompetitionStage = 'CompetitionStage'
}Committable suggestion skipped: line range outside the PR's diff.
| export enum CompetitionDetailsTabTitlesParams { | ||
| 'AboutCompetition', | ||
| 'Judges', | ||
| 'Contacts' | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using string values for tab title parameters.
The current implementation uses numeric enum values (0, 1, 2), which could make internationalization and maintenance more difficult.
Consider using explicit string values:
export enum CompetitionDetailsTabTitlesParams {
- 'AboutCompetition',
- 'Judges',
- 'Contacts'
+ AboutCompetition = 'AboutCompetition',
+ Judges = 'Judges',
+ Contacts = 'Contacts'
}This change would:
- Make the values more explicit and self-documenting
- Facilitate easier internationalization
- Maintain consistency with other enums in the file
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export enum CompetitionDetailsTabTitlesParams { | |
| 'AboutCompetition', | |
| 'Judges', | |
| 'Contacts' | |
| } | |
| export enum CompetitionDetailsTabTitlesParams { | |
| AboutCompetition = 'AboutCompetition', | |
| Judges = 'Judges', | |
| Contacts = 'Contacts' | |
| } |
| it('should create', () => { | ||
| expect(component).toBeTruthy(); | ||
| }); | ||
|
|
||
| it('should set navigation path', () => { | ||
| component.addNavPath(); | ||
| expect(storeMock.dispatch).toHaveBeenCalledWith(new PushNavPath({ name: expect.any(String), isActive: false, disable: true })); | ||
| }); | ||
|
|
||
| it('should change page and fetch competitions', () => { | ||
| const page = { element: 2, isActive: false }; | ||
| component.onPageChange(page); | ||
| expect(component.currentPage).toBe(page); | ||
| }); | ||
|
|
||
| it('should change items per page and reset to first page', () => { | ||
| component.onItemsPerPageChange(20); | ||
| expect(component.competitionCardParameters.size).toBe(20); | ||
| expect(component.currentPage).toBe(PaginationConstants.firstPage); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add test coverage for missing scenarios.
The current test suite should be expanded to include:
- Error handling scenarios
- Edge cases in pagination
- Competition deletion functionality
- State changes and their effects
Add these test cases:
it('should handle pagination edge cases', () => {
const invalidPage = { element: -1, isActive: false };
component.onPageChange(invalidPage);
expect(component.currentPage).toBe(PaginationConstants.firstPage);
});
it('should handle competition deletion', () => {
const competitionId = '123';
component.onDelete(competitionId);
expect(matDialogMock.open).toHaveBeenCalled();
// Verify deletion action dispatch
});
it('should handle store errors', () => {
storeMock.select.mockReturnValue(throwError(() => new Error('Store error')));
// Verify error handling
});
it('should update view when competition state changes', () => {
const mockCompetitions = { entities: [{ id: '1', name: 'Test' }], totalAmount: 1 };
storeMock.select.mockReturnValue(of(mockCompetitions));
fixture.detectChanges();
// Verify view updates
});| this.optionsForPeopleWithDisabilities = Boolean(description.disabilityOptionsDesc); | ||
| this.competitiveSelection = Boolean(description.selectionOptionsDesc); | ||
| this.areThereBenefits = Boolean(description.benefitsOptionsDesc); | ||
| this.optionsForPeopleWithDisabilities = description.optionsForPeopleWithDisabilities; |
There was a problem hiding this comment.
Overwritten property assignment.
Line 71 sets optionsForPeopleWithDisabilities to a boolean check of disabilityOptionsDesc, then line 74 immediately overwrites it with description.optionsForPeopleWithDisabilities. This duplicate assignment can cause confusion and potential bugs. Consider removing the first assignment or consolidating the logic into a single statement.
- this.optionsForPeopleWithDisabilities = Boolean(description.disabilityOptionsDesc);
- ...
this.optionsForPeopleWithDisabilities = description.optionsForPeopleWithDisabilities;
+ this.optionsForPeopleWithDisabilities = description.optionsForPeopleWithDisabilities ?? Boolean(description.disabilityOptionsDesc);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this.optionsForPeopleWithDisabilities = Boolean(description.disabilityOptionsDesc); | |
| this.competitiveSelection = Boolean(description.selectionOptionsDesc); | |
| this.areThereBenefits = Boolean(description.benefitsOptionsDesc); | |
| this.optionsForPeopleWithDisabilities = description.optionsForPeopleWithDisabilities; | |
| this.competitiveSelection = Boolean(description.selectionOptionsDesc); | |
| this.areThereBenefits = Boolean(description.benefitsOptionsDesc); | |
| this.optionsForPeopleWithDisabilities = description.optionsForPeopleWithDisabilities ?? Boolean(description.disabilityOptionsDesc); |
| describe('UserCompetitionService', () => { | ||
| let service: UserCompetitionService; | ||
|
|
||
| beforeEach(() => { | ||
| TestBed.configureTestingModule({}); | ||
| service = TestBed.inject(UserCompetitionService); | ||
| }); | ||
|
|
||
| it('should be created', () => { | ||
| expect(service).toBeTruthy(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance test coverage for UserCompetitionService.
While the basic test setup is correct, the test suite only verifies service creation. Consider adding tests for the following service methods mentioned in the AI summary:
- getProviderViewCompetitions
- createCompetition
- deleteCompetitionById
Would you like me to help generate additional test cases for these methods?
| public ngOnInit(): void { | ||
| this.initForm(); | ||
| this.PassRequiredFormGroup.emit(this.RequiredFormGroup); | ||
|
|
||
| this.filterTypeOfCompetition(Boolean(this.parentCompetition)); | ||
|
|
||
| if (this.competition) { | ||
| this.activateEditMode(); | ||
| } | ||
| if (this.parentCompetition) { | ||
| this.typeOfCompetitionControl.setValue(TypeOfCompetition.CompetitionStage); | ||
| this.typeOfCompetitionControl.disable(); | ||
| } | ||
|
|
||
| this.initListeners(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for date operations.
Date parsing and form operations should include error handling to prevent runtime errors.
Example implementation:
public ngOnInit(): void {
try {
this.initForm();
this.PassRequiredFormGroup.emit(this.RequiredFormGroup);
this.filterTypeOfCompetition(Boolean(this.parentCompetition));
if (this.competition) {
this.activateEditMode();
}
if (this.parentCompetition) {
this.typeOfCompetitionControl.setValue(TypeOfCompetition.CompetitionStage);
this.typeOfCompetitionControl.disable();
}
this.initListeners();
} catch (error) {
console.error('Failed to initialize form:', error);
// Handle error appropriately
}
}
src/app/shared/shared.module.ts
Outdated
| ScrollToTopComponent, | ||
| CompetitionCardComponent |
There was a problem hiding this comment.
Remove duplicate ScrollToTopComponent declaration.
The ScrollToTopComponent is already declared at line 115, making this declaration redundant. Angular modules should declare components only once.
Apply this diff to fix the duplicate declaration:
- ScrollToTopComponent,
CompetitionCardComponent📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ScrollToTopComponent, | |
| CompetitionCardComponent | |
| CompetitionCardComponent |
| public static getCompetitionMessage(payload: Competition & any, message: string): MessageBarData { | ||
| const finalMessage: MessageBarData = { message: '', type: 'success' }; | ||
| const messageArr = []; | ||
| let isInvalidCoverImage = false; | ||
| let isInvalidGalleryImages = false; | ||
| let statuses; | ||
| let invalidImages; | ||
|
|
||
| if (payload.uploadingCoverImageResult) { | ||
| isInvalidCoverImage = !payload.uploadingCoverImageResult.result.succeeded; | ||
| } | ||
|
|
||
| if (payload.uploadingImagesResults?.results) { | ||
| statuses = Object.entries(payload.uploadingImagesResults.results); | ||
| invalidImages = statuses.filter((result) => !result[1].succeeded); | ||
| isInvalidGalleryImages = !!invalidImages.length; | ||
| } | ||
|
|
||
| messageArr.push(message); | ||
|
|
||
| if (isInvalidCoverImage) { | ||
| const coverImageErrorMsg = payload.uploadingCoverImageResult?.result.errors | ||
| .map((error) => `"${CodeMessageErrors[error.code]}"`) | ||
| .join(', '); | ||
|
|
||
| messageArr.push(`Помилка завантаження фонового зображення: ${coverImageErrorMsg}`); | ||
| finalMessage.type = 'warningYellow'; | ||
| } | ||
|
|
||
| if (isInvalidGalleryImages) { | ||
| const errorCodes = new Set(); | ||
| invalidImages.map((img) => img[1]).forEach((img) => img.errors.forEach((error) => errorCodes.add(error.code))); | ||
| const errorMsg = [...errorCodes].map((error: string) => `"${CodeMessageErrors[error]}"`).join(', '); | ||
| const indexes = invalidImages.map((img) => img[0]); | ||
| const quantityMsg = indexes.length > 1 ? `у ${indexes.length} зображень` : `у ${+indexes[0] + 1}-го зображення`; | ||
|
|
||
| messageArr.push(`Помилка завантаження ${quantityMsg} для галереї: ${errorMsg}`); | ||
| finalMessage.type = 'warningYellow'; | ||
| } | ||
|
|
||
| finalMessage.message = messageArr.join(';\n'); | ||
|
|
||
| return finalMessage; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor to eliminate code duplication with getWorkshopMessage.
The getCompetitionMessage function duplicates the logic from getWorkshopMessage. Consider extracting the shared image handling logic into a reusable function.
Apply this diff to reduce code duplication:
+ private static getImageUploadMessage(payload: any, message: string): MessageBarData {
+ const finalMessage: MessageBarData = { message: '', type: 'success' };
+ const messageArr = [];
+ let isInvalidCoverImage = false;
+ let isInvalidGalleryImages = false;
+ let statuses;
+ let invalidImages;
+
+ if (payload.uploadingCoverImageResult) {
+ isInvalidCoverImage = !payload.uploadingCoverImageResult.result.succeeded;
+ }
+
+ if (payload.uploadingImagesResults?.results) {
+ statuses = Object.entries(payload.uploadingImagesResults.results);
+ invalidImages = statuses.filter((result) => !result[1].succeeded);
+ isInvalidGalleryImages = !!invalidImages.length;
+ }
+
+ messageArr.push(message);
+
+ if (isInvalidCoverImage) {
+ const coverImageErrorMsg = payload.uploadingCoverImageResult?.result.errors
+ .map((error) => `"${CodeMessageErrors[error.code]}"`)
+ .join(', ');
+
+ messageArr.push(`Помилка завантаження фонового зображення: ${coverImageErrorMsg}`);
+ finalMessage.type = 'warningYellow';
+ }
+
+ if (isInvalidGalleryImages) {
+ const errorCodes = new Set();
+ invalidImages.map((img) => img[1]).forEach((img) => img.errors.forEach((error) => errorCodes.add(error.code)));
+ const errorMsg = [...errorCodes].map((error: string) => `"${CodeMessageErrors[error]}"`).join(', ');
+ const indexes = invalidImages.map((img) => img[0]);
+ const quantityMsg = indexes.length > 1 ? `у ${indexes.length} зображень` : `у ${+indexes[0] + 1}-го зображення`;
+
+ messageArr.push(`Помилка завантаження ${quantityMsg} для галереї: ${errorMsg}`);
+ finalMessage.type = 'warningYellow';
+ }
+
+ finalMessage.message = messageArr.join(';\n');
+
+ return finalMessage;
+ }
public static getWorkshopMessage(payload: Workshop & any, message: string): MessageBarData {
- const finalMessage: MessageBarData = { message: '', type: 'success' };
- const messageArr = [];
- let isInvalidCoverImage = false;
- let isInvalidGalleryImages = false;
- let statuses;
- let invalidImages;
-
- if (payload.uploadingCoverImageResult) {
- isInvalidCoverImage = !payload.uploadingCoverImageResult.result.succeeded;
- }
-
- if (payload.uploadingImagesResults?.results) {
- statuses = Object.entries(payload.uploadingImagesResults.results);
- invalidImages = statuses.filter((result) => !result[1].succeeded);
- isInvalidGalleryImages = !!invalidImages.length;
- }
-
- messageArr.push(message);
-
- if (isInvalidCoverImage) {
- const coverImageErrorMsg = payload.uploadingCoverImageResult?.result.errors
- .map((error) => `"${CodeMessageErrors[error.code]}"`)
- .join(', ');
-
- messageArr.push(`Помилка завантаження фонового зображення: ${coverImageErrorMsg}`);
- finalMessage.type = 'warningYellow';
- }
-
- if (isInvalidGalleryImages) {
- const errorCodes = new Set();
- invalidImages.map((img) => img[1]).forEach((img) => img.errors.forEach((error) => errorCodes.add(error.code)));
- const errorMsg = [...errorCodes].map((error: string) => `"${CodeMessageErrors[error]}"`).join(', ');
- const indexes = invalidImages.map((img) => img[0]);
- const quantityMsg = indexes.length > 1 ? `у ${indexes.length} зображень` : `у ${+indexes[0] + 1}-го зображення`;
-
- messageArr.push(`Помилка завантаження ${quantityMsg} для галереї: ${errorMsg}`);
- finalMessage.type = 'warningYellow';
- }
-
- finalMessage.message = messageArr.join(';\n');
-
- return finalMessage;
+ return this.getImageUploadMessage(payload, message);
}
public static getCompetitionMessage(payload: Competition & any, message: string): MessageBarData {
- const finalMessage: MessageBarData = { message: '', type: 'success' };
- const messageArr = [];
- let isInvalidCoverImage = false;
- let isInvalidGalleryImages = false;
- let statuses;
- let invalidImages;
-
- if (payload.uploadingCoverImageResult) {
- isInvalidCoverImage = !payload.uploadingCoverImageResult.result.succeeded;
- }
-
- if (payload.uploadingImagesResults?.results) {
- statuses = Object.entries(payload.uploadingImagesResults.results);
- invalidImages = statuses.filter((result) => !result[1].succeeded);
- isInvalidGalleryImages = !!invalidImages.length;
- }
-
- messageArr.push(message);
-
- if (isInvalidCoverImage) {
- const coverImageErrorMsg = payload.uploadingCoverImageResult?.result.errors
- .map((error) => `"${CodeMessageErrors[error.code]}"`)
- .join(', ');
-
- messageArr.push(`Помилка завантаження фонового зображення: ${coverImageErrorMsg}`);
- finalMessage.type = 'warningYellow';
- }
-
- if (isInvalidGalleryImages) {
- const errorCodes = new Set();
- invalidImages.map((img) => img[1]).forEach((img) => img.errors.forEach((error) => errorCodes.add(error.code)));
- const errorMsg = [...errorCodes].map((error: string) => `"${CodeMessageErrors[error]}"`).join(', ');
- const indexes = invalidImages.map((img) => img[0]);
- const quantityMsg = indexes.length > 1 ? `у ${indexes.length} зображень` : `у ${+indexes[0] + 1}-го зображення`;
-
- messageArr.push(`Помилка завантаження ${quantityMsg} для галереї: ${errorMsg}`);
- finalMessage.type = 'warningYellow';
- }
-
- finalMessage.message = messageArr.join(';\n');
-
- return finalMessage;
+ return this.getImageUploadMessage(payload, message);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public static getCompetitionMessage(payload: Competition & any, message: string): MessageBarData { | |
| const finalMessage: MessageBarData = { message: '', type: 'success' }; | |
| const messageArr = []; | |
| let isInvalidCoverImage = false; | |
| let isInvalidGalleryImages = false; | |
| let statuses; | |
| let invalidImages; | |
| if (payload.uploadingCoverImageResult) { | |
| isInvalidCoverImage = !payload.uploadingCoverImageResult.result.succeeded; | |
| } | |
| if (payload.uploadingImagesResults?.results) { | |
| statuses = Object.entries(payload.uploadingImagesResults.results); | |
| invalidImages = statuses.filter((result) => !result[1].succeeded); | |
| isInvalidGalleryImages = !!invalidImages.length; | |
| } | |
| messageArr.push(message); | |
| if (isInvalidCoverImage) { | |
| const coverImageErrorMsg = payload.uploadingCoverImageResult?.result.errors | |
| .map((error) => `"${CodeMessageErrors[error.code]}"`) | |
| .join(', '); | |
| messageArr.push(`Помилка завантаження фонового зображення: ${coverImageErrorMsg}`); | |
| finalMessage.type = 'warningYellow'; | |
| } | |
| if (isInvalidGalleryImages) { | |
| const errorCodes = new Set(); | |
| invalidImages.map((img) => img[1]).forEach((img) => img.errors.forEach((error) => errorCodes.add(error.code))); | |
| const errorMsg = [...errorCodes].map((error: string) => `"${CodeMessageErrors[error]}"`).join(', '); | |
| const indexes = invalidImages.map((img) => img[0]); | |
| const quantityMsg = indexes.length > 1 ? `у ${indexes.length} зображень` : `у ${+indexes[0] + 1}-го зображення`; | |
| messageArr.push(`Помилка завантаження ${quantityMsg} для галереї: ${errorMsg}`); | |
| finalMessage.type = 'warningYellow'; | |
| } | |
| finalMessage.message = messageArr.join(';\n'); | |
| return finalMessage; | |
| } | |
| private static getImageUploadMessage(payload: any, message: string): MessageBarData { | |
| const finalMessage: MessageBarData = { message: '', type: 'success' }; | |
| const messageArr = []; | |
| let isInvalidCoverImage = false; | |
| let isInvalidGalleryImages = false; | |
| let statuses; | |
| let invalidImages; | |
| if (payload.uploadingCoverImageResult) { | |
| isInvalidCoverImage = !payload.uploadingCoverImageResult.result.succeeded; | |
| } | |
| if (payload.uploadingImagesResults?.results) { | |
| statuses = Object.entries(payload.uploadingImagesResults.results); | |
| invalidImages = statuses.filter((result) => !result[1].succeeded); | |
| isInvalidGalleryImages = !!invalidImages.length; | |
| } | |
| messageArr.push(message); | |
| if (isInvalidCoverImage) { | |
| const coverImageErrorMsg = payload.uploadingCoverImageResult?.result.errors | |
| .map((error) => `"${CodeMessageErrors[error.code]}"`) | |
| .join(', '); | |
| messageArr.push(`Помилка завантаження фонового зображення: ${coverImageErrorMsg}`); | |
| finalMessage.type = 'warningYellow'; | |
| } | |
| if (isInvalidGalleryImages) { | |
| const errorCodes = new Set(); | |
| invalidImages.map((img) => img[1]).forEach((img) => img.errors.forEach((error) => errorCodes.add(error.code))); | |
| const errorMsg = [...errorCodes].map((error: string) => `"${CodeMessageErrors[error]}"`).join(', '); | |
| const indexes = invalidImages.map((img) => img[0]); | |
| const quantityMsg = indexes.length > 1 ? `у ${indexes.length} зображень` : `у ${+indexes[0] + 1}-го зображення`; | |
| messageArr.push(`Помилка завантаження ${quantityMsg} для галереї: ${errorMsg}`); | |
| finalMessage.type = 'warningYellow'; | |
| } | |
| finalMessage.message = messageArr.join(';\n'); | |
| return finalMessage; | |
| } | |
| public static getWorkshopMessage(payload: Workshop & any, message: string): MessageBarData { | |
| - const finalMessage: MessageBarData = { message: '', type: 'success' }; | |
| - const messageArr = []; | |
| - let isInvalidCoverImage = false; | |
| - let isInvalidGalleryImages = false; | |
| - let statuses; | |
| - let invalidImages; | |
| - | |
| - if (payload.uploadingCoverImageResult) { | |
| - isInvalidCoverImage = !payload.uploadingCoverImageResult.result.succeeded; | |
| - } | |
| - | |
| - if (payload.uploadingImagesResults?.results) { | |
| - statuses = Object.entries(payload.uploadingImagesResults.results); | |
| - invalidImages = statuses.filter((result) => !result[1].succeeded); | |
| - isInvalidGalleryImages = !!invalidImages.length; | |
| - } | |
| - | |
| - messageArr.push(message); | |
| - | |
| - if (isInvalidCoverImage) { | |
| - const coverImageErrorMsg = payload.uploadingCoverImageResult?.result.errors | |
| - .map((error) => `"${CodeMessageErrors[error.code]}"`) | |
| - .join(', '); | |
| - | |
| - messageArr.push(`Помилка завантаження фонового зображення: ${coverImageErrorMsg}`); | |
| - finalMessage.type = 'warningYellow'; | |
| - } | |
| - | |
| - if (isInvalidGalleryImages) { | |
| - const errorCodes = new Set(); | |
| - invalidImages.map((img) => img[1]).forEach((img) => img.errors.forEach((error) => errorCodes.add(error.code))); | |
| - const errorMsg = [...errorCodes].map((error: string) => `"${CodeMessageErrors[error]}"`).join(', '); | |
| - const indexes = invalidImages.map((img) => img[0]); | |
| - const quantityMsg = indexes.length > 1 ? `у ${indexes.length} зображень` : `у ${+indexes[0] + 1}-го зображення`; | |
| - | |
| - messageArr.push(`Помилка завантаження ${quantityMsg} для галереї: ${errorMsg}`); | |
| - finalMessage.type = 'warningYellow'; | |
| - } | |
| - | |
| - finalMessage.message = messageArr.join(';\n'); | |
| - | |
| - return finalMessage; | |
| + return this.getImageUploadMessage(payload, message); | |
| } | |
| public static getCompetitionMessage(payload: Competition & any, message: string): MessageBarData { | |
| - const finalMessage: MessageBarData = { message: '', type: 'success' }; | |
| - const messageArr = []; | |
| - let isInvalidCoverImage = false; | |
| - let isInvalidGalleryImages = false; | |
| - let statuses; | |
| - let invalidImages; | |
| - | |
| - if (payload.uploadingCoverImageResult) { | |
| - isInvalidCoverImage = !payload.uploadingCoverImageResult.result.succeeded; | |
| - } | |
| - | |
| - if (payload.uploadingImagesResults?.results) { | |
| - statuses = Object.entries(payload.uploadingImagesResults.results); | |
| - invalidImages = statuses.filter((result) => !result[1].succeeded); | |
| - isInvalidGalleryImages = !!invalidImages.length; | |
| - } | |
| - | |
| - messageArr.push(message); | |
| - | |
| - if (isInvalidCoverImage) { | |
| - const coverImageErrorMsg = payload.uploadingCoverImageResult?.result.errors | |
| - .map((error) => `"${CodeMessageErrors[error.code]}"`) | |
| - .join(', '); | |
| - | |
| - messageArr.push(`Помилка завантаження фонового зображення: ${coverImageErrorMsg}`); | |
| - finalMessage.type = 'warningYellow'; | |
| - } | |
| - | |
| - if (isInvalidGalleryImages) { | |
| - const errorCodes = new Set(); | |
| - invalidImages.map((img) => img[1]).forEach((img) => img.errors.forEach((error) => errorCodes.add(error.code))); | |
| - const errorMsg = [...errorCodes].map((error: string) => `"${CodeMessageErrors[error]}"`).join(', '); | |
| - const indexes = invalidImages.map((img) => img[0]); | |
| - const quantityMsg = indexes.length > 1 ? `у ${indexes.length} зображень` : `у ${+indexes[0] + 1}-го зображення`; | |
| - | |
| - messageArr.push(`Помилка завантаження ${quantityMsg} для галереї: ${errorMsg}`); | |
| - finalMessage.type = 'warningYellow'; | |
| - } | |
| - | |
| - finalMessage.message = messageArr.join(';\n'); | |
| - | |
| - return finalMessage; | |
| + return this.getImageUploadMessage(payload, message); | |
| } |
| @Action(providerActions.DeleteCompetitionByIdSuccess) | ||
| deleteCompetitionByIdSuccess( | ||
| { dispatch }: StateContext<ProviderStateModel>, | ||
| { competition }: providerActions.DeleteCompetitionByIdSuccess | ||
| ): void { | ||
| const messageData = Util.getCompetitionMessage(competition, SnackbarText.deleteCompetition); | ||
| dispatch([new MarkFormDirty(false), new ShowMessageBar({ message: messageData.message, type: messageData.type })]); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider refreshing the competition list after deletion.
The deleteCompetitionByIdSuccess action only shows a message but doesn't refresh the competition list. Consider dispatching a GetProviderViewCompetitions action to refresh the list.
Apply this diff to refresh the list after deletion:
deleteCompetitionByIdSuccess(
{ dispatch }: StateContext<ProviderStateModel>,
- { competition }: providerActions.DeleteCompetitionByIdSuccess
+ { competition, parameters }: providerActions.DeleteCompetitionByIdSuccess
): void {
const messageData = Util.getCompetitionMessage(competition, SnackbarText.deleteCompetition);
- dispatch([new MarkFormDirty(false), new ShowMessageBar({ message: messageData.message, type: messageData.type })]);
+ dispatch([
+ new MarkFormDirty(false),
+ new ShowMessageBar({ message: messageData.message, type: messageData.type }),
+ new providerActions.GetProviderViewCompetitions(parameters)
+ ]);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Action(providerActions.DeleteCompetitionByIdSuccess) | |
| deleteCompetitionByIdSuccess( | |
| { dispatch }: StateContext<ProviderStateModel>, | |
| { competition }: providerActions.DeleteCompetitionByIdSuccess | |
| ): void { | |
| const messageData = Util.getCompetitionMessage(competition, SnackbarText.deleteCompetition); | |
| dispatch([new MarkFormDirty(false), new ShowMessageBar({ message: messageData.message, type: messageData.type })]); | |
| } | |
| @Action(providerActions.DeleteCompetitionByIdSuccess) | |
| deleteCompetitionByIdSuccess( | |
| { dispatch }: StateContext<ProviderStateModel>, | |
| { competition, parameters }: providerActions.DeleteCompetitionByIdSuccess | |
| ): void { | |
| const messageData = Util.getCompetitionMessage(competition, SnackbarText.deleteCompetition); | |
| dispatch([ | |
| new MarkFormDirty(false), | |
| new ShowMessageBar({ message: messageData.message, type: messageData.type }), | |
| new providerActions.GetProviderViewCompetitions(parameters) | |
| ]); | |
| } |
| <div class="map"> | ||
| <app-map [settelmentFormGroup]="settlementFormControl" [addressFormGroup]="addressFormGroup" (addressSelect)="onAddressSelect($event)"> | ||
| </app-map> | ||
| </div> |
There was a problem hiding this comment.
Potential typo in map component input.
The component uses the input attribute [settelmentFormGroup] with a value bound to settlementFormControl. If the intended property name is “settlementFormGroup” (with an extra “t”), please correct this typo to ensure consistency and prevent runtime issues.
DimaShapoval
force-pushed
the
Romanchak/Create-competitive-event
branch
from
a23cf28 to
55cdc30
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
src/app/shared/models/competition.model.ts (1)
71-74:⚠️ Potential issueFix duplicate property assignment.
The property
optionsForPeopleWithDisabilitiesis assigned twice, which can lead to confusion and potential bugs.Apply this diff to fix the duplicate assignment:
- this.optionsForPeopleWithDisabilities = Boolean(description.disabilityOptionsDesc); - ... this.optionsForPeopleWithDisabilities = description.optionsForPeopleWithDisabilities; + this.optionsForPeopleWithDisabilities = description.optionsForPeopleWithDisabilities ?? Boolean(description.disabilityOptionsDesc);
🧹 Nitpick comments (11)
src/app/shared/services/images/images.service.ts (1)
30-31: Use a dedicated default image for providers.
The fallback logic for all non-Workshop entities points to a competition image, which might not be suitable for providers without images. Consider specifying a unique provider default image.src/app/shell/personal-cabinet/provider/create-competition/create-judge/judge-form/judge-form.component.ts (1)
27-28: Remove the unused constructor.
The empty constructor adds no value and can be removed to reduce code complexity.- constructor() {}🧰 Tools
🪛 Biome (1.9.4)
[error] 27-27: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
src/app/shared/store/shared-user.actions.ts (1)
84-87: Remove unnecessary constructor.The empty constructor can be removed as it doesn't add any functionality.
export class ResetProviderCompetitionDetails { static readonly type = '[user] clear Provider And Competition Details'; - constructor() {} }🧰 Tools
🪛 Biome (1.9.4)
[error] 86-86: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
src/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.ts (1)
71-74: Enhance method documentation.The current documentation could be more descriptive about when and why the form needs to be marked as dirty.
/** - * This method makes addressFormGroup dirty + * Marks the address form as dirty when the user interacts with the settlement search, + * ensuring that the form's state correctly reflects user modifications even when + * values are set programmatically from the geocoding result. */src/app/shared/components/competition-card/competition-card.component.ts (1)
55-61: Simplify role subscription logic.The role subscription can be simplified using the
tapoperator.this.role$ .pipe(takeUntil(this.destroy$)) .pipe(filter((role: Role) => role === Role.parent)) - .subscribe((role: Role) => { - this.role = role; - }); + .pipe(tap((role: Role) => this.role = role)) + .subscribe();src/app/shell/details/details.component.ts (1)
92-94: Update method documentation.The method documentation should be updated to include competition handling.
/** - * This method get Workshop or Provider by Id; + * Gets Workshop, Competition, or Provider by Id based on the entity type. */src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.ts (1)
100-102: Remove commented code.Remove the commented code block as it's not being used and adds unnecessary noise to the codebase.
- // else { - // this.store.dispatch(new GetEmployeeWorkshops(this.workshopCardParameters)); - // }src/app/shared/store/shared-user.state.ts (2)
113-113: Replace void with undefined in union type.Using void in a union type can be confusing. Consider using undefined instead.
Apply this diff to fix the type:
- ): Observable<Competition | void> { + ): Observable<Competition | undefined> {🧰 Tools
🪛 Biome (1.9.4)
[error] 113-113: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
147-151: Improve error handling with specific error messages.The error handling uses a generic error message. Consider using more specific error messages based on the error type.
Apply this diff to improve error handling:
@Action(OnGetCompetitionByIdFail) onGetCompetitionByIdFail({ dispatch, patchState }: StateContext<SharedUserStateModel>, { payload }: OnGetCompetitionByIdFail): void { patchState({ selectedCompetition: null, isLoading: false }); - dispatch(new ShowMessageBar({ message: SnackbarText.error, type: 'error' })); + const errorMessage = payload?.error?.message || SnackbarText.error; + dispatch(new ShowMessageBar({ message: errorMessage, type: 'error' })); }src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.ts (1)
235-238: Simplify the seat availability control logic.The
setAvailableSeatsControlValuemethod could be simplified by using a more declarative approach.- private setAvailableSeatsControlValue(availableSeats: number = null, action: string = 'disable', emitEvent: boolean = true): void { - this.availableSeatsControl[action]({ emitEvent }); - this.availableSeatsControl.setValue(availableSeats, { emitEvent }); + private setAvailableSeatsControlValue(availableSeats: number | null = null, enabled: boolean = false, emitEvent: boolean = true): void { + enabled ? this.availableSeatsControl.enable({ emitEvent }) : this.availableSeatsControl.disable({ emitEvent }); + this.availableSeatsControl.setValue(availableSeats, { emitEvent }); }src/app/shell/personal-cabinet/provider/create-competition/create-judge/judge-form/judge-form.component.html (1)
1-101: Enhance form accessibility with ARIA attributes.While the form has good accessibility features, it could be improved with ARIA attributes for better screen reader support.
Apply these changes to enhance accessibility:
-<form [formGroup]="JudgeForm" class="flex flex-col justify-center items-between step"> +<form [formGroup]="JudgeForm" class="flex flex-col justify-center items-between step" role="form" aria-label="{{ 'FORMS.LABELS.JUDGE_FORM' | translate }}"> <mat-checkbox class="step-label" color="primary" formControlName="isChiefJudge" (focusout)="onFocusOut('isChiefJudge')" id="is-chief-judge" + aria-describedby="chief-judge-hint" >{{ 'FORMS.LABELS.CHIEF_JUDGE' | translate }}</mat-checkbox> -<app-validation-hint [validationFormControl]="JudgeFormGroup.get('isChiefJudge')"> </app-validation-hint> +<app-validation-hint [validationFormControl]="JudgeFormGroup.get('isChiefJudge')" id="chief-judge-hint"> </app-validation-hint>Add similar
aria-describedbyattributes to other form controls, linking them to their respective validation hints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/assets/i18n/en.jsonis excluded by!src/assets/**and included bysrc/**src/assets/i18n/uk.jsonis excluded by!src/assets/**and included bysrc/**
📒 Files selected for processing (83)
src/app/header/header.component.html(1 hunks)src/app/shared/components/competition-card/competition-card.component.html(1 hunks)src/app/shared/components/competition-card/competition-card.component.scss(1 hunks)src/app/shared/components/competition-card/competition-card.component.spec.ts(1 hunks)src/app/shared/components/competition-card/competition-card.component.ts(1 hunks)src/app/shared/components/sidenav-menu/sidenav-menu.component.html(1 hunks)src/app/shared/components/validation-hint/validation-hint.component.html(2 hunks)src/app/shared/components/validation-hint/validation-hint.component.ts(2 hunks)src/app/shared/components/workshop-card/workshop-card.component.ts(1 hunks)src/app/shared/constants/constants.ts(3 hunks)src/app/shared/constants/validation.ts(1 hunks)src/app/shared/enum/competition.ts(1 hunks)src/app/shared/enum/enumUA/competition.ts(1 hunks)src/app/shared/enum/enumUA/message-bar.ts(1 hunks)src/app/shared/enum/enumUA/navigation-bar.ts(1 hunks)src/app/shared/enum/enumUA/no-results.ts(1 hunks)src/app/shared/enum/modal-confirmation.ts(6 hunks)src/app/shared/models/address.model.ts(2 hunks)src/app/shared/models/codeficator.model.ts(1 hunks)src/app/shared/models/competition.model.ts(1 hunks)src/app/shared/models/institution.model.ts(1 hunks)src/app/shared/models/judge.model.ts(1 hunks)src/app/shared/models/provider.model.ts(1 hunks)src/app/shared/services/competitions/user-competition.service.spec.ts(1 hunks)src/app/shared/services/competitions/user-competition.service.ts(1 hunks)src/app/shared/services/images/images.service.ts(2 hunks)src/app/shared/shared.module.ts(3 hunks)src/app/shared/store/provider.actions.ts(3 hunks)src/app/shared/store/provider.state.ts(7 hunks)src/app/shared/store/shared-user.actions.ts(3 hunks)src/app/shared/store/shared-user.state.ts(7 hunks)src/app/shared/styles/create-form.scss(1 hunks)src/app/shared/utils/utils.ts(2 hunks)src/app/shell/details/competition-details/competition-details.component.html(1 hunks)src/app/shell/details/competition-details/competition-details.component.scss(1 hunks)src/app/shell/details/competition-details/competition-details.component.spec.ts(1 hunks)src/app/shell/details/competition-details/competition-details.component.ts(1 hunks)src/app/shell/details/details-tabs/competition-about/competition-about.component.html(1 hunks)src/app/shell/details/details-tabs/competition-about/competition-about.component.scss(1 hunks)src/app/shell/details/details-tabs/competition-about/competition-about.component.spec.ts(1 hunks)src/app/shell/details/details-tabs/competition-about/competition-about.component.ts(1 hunks)src/app/shell/details/details-tabs/competition-judges/competition-judges.component.html(1 hunks)src/app/shell/details/details-tabs/competition-judges/competition-judges.component.scss(1 hunks)src/app/shell/details/details-tabs/competition-judges/competition-judges.component.spec.ts(1 hunks)src/app/shell/details/details-tabs/competition-judges/competition-judges.component.ts(1 hunks)src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.html(1 hunks)src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.scss(1 hunks)src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.spec.ts(1 hunks)src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.ts(1 hunks)src/app/shell/details/details.component.html(2 hunks)src/app/shell/details/details.component.ts(5 hunks)src/app/shell/details/details.module.ts(2 hunks)src/app/shell/details/side-menu/side-menu.component.ts(2 hunks)src/app/shell/personal-cabinet/personal-cabinet.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.scss(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.scss(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.scss(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-judge/create-judge.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-judge/create-judge.component.scss(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-judge/create-judge.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-judge/judge-form/judge-form.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-judge/judge-form/judge-form.component.scss(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-judge/judge-form/judge-form.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.scss(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-workshop/create-about-form/create-about-form.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-workshop/create-workshop.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-workshop/create-workshop.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.html(1 hunks)src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.scss(1 hunks)src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.spec.ts(1 hunks)src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/provider-routing.module.ts(2 hunks)src/app/shell/personal-cabinet/provider/provider.module.ts(2 hunks)src/app/shell/personal-cabinet/shared-cabinet/applications/application-card/application-card.component.html(1 hunks)src/app/shell/shell-routing.module.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (64)
- src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.scss
- src/app/shell/personal-cabinet/provider/create-competition/create-judge/judge-form/judge-form.component.scss
- src/app/shared/models/provider.model.ts
- src/app/shell/details/details-tabs/competition-judges/competition-judges.component.scss
- src/app/shared/models/codeficator.model.ts
- src/app/shell/personal-cabinet/provider/create-competition/create-judge/create-judge.component.scss
- src/app/shared/services/competitions/user-competition.service.spec.ts
- src/app/shared/models/institution.model.ts
- src/app/shared/components/sidenav-menu/sidenav-menu.component.html
- src/app/shared/enum/enumUA/no-results.ts
- src/app/shell/details/details-tabs/competition-about/competition-about.component.ts
- src/app/shell/details/details-tabs/competition-judges/competition-judges.component.html
- src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.scss
- src/app/shared/styles/create-form.scss
- src/app/shell/details/details-tabs/competition-about/competition-about.component.spec.ts
- src/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.scss
- src/app/shell/personal-cabinet/provider/create-workshop/create-about-form/create-about-form.component.ts
- src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.scss
- src/app/shared/models/address.model.ts
- src/app/shared/components/validation-hint/validation-hint.component.ts
- src/app/shell/details/details-tabs/competition-judges/competition-judges.component.spec.ts
- src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.spec.ts
- src/app/shared/components/validation-hint/validation-hint.component.html
- src/app/header/header.component.html
- src/app/shell/personal-cabinet/provider/create-workshop/create-workshop.component.ts
- src/app/shared/shared.module.ts
- src/app/shared/enum/enumUA/competition.ts
- src/app/shell/personal-cabinet/provider/create-competition/create-judge/create-judge.component.html
- src/app/shell/details/details-tabs/competition-judges/competition-judges.component.ts
- src/app/shell/personal-cabinet/shared-cabinet/applications/application-card/application-card.component.html
- src/app/shared/components/competition-card/competition-card.component.html
- src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.html
- src/app/shell/details/details.module.ts
- src/app/shell/personal-cabinet/provider/create-workshop/create-workshop.component.html
- src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.scss
- src/app/shell/details/details-tabs/competition-about/competition-about.component.html
- src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.html
- src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.html
- src/app/shell/details/details.component.html
- src/app/shared/components/workshop-card/workshop-card.component.ts
- src/app/shared/models/judge.model.ts
- src/app/shell/details/competition-details/competition-details.component.scss
- src/app/shared/constants/validation.ts
- src/app/shell/details/side-menu/side-menu.component.ts
- src/app/shell/personal-cabinet/provider/provider-routing.module.ts
- src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.html
- src/app/shell/details/details-tabs/competition-about/competition-about.component.scss
- src/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.html
- src/app/shell/details/competition-details/competition-details.component.html
- src/app/shared/constants/constants.ts
- src/app/shared/components/competition-card/competition-card.component.scss
- src/app/shell/personal-cabinet/personal-cabinet.component.html
- src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.spec.ts
- src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.html
- src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.scss
- src/app/shared/enum/competition.ts
- src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.ts
- src/app/shared/enum/enumUA/message-bar.ts
- src/app/shared/enum/enumUA/navigation-bar.ts
- src/app/shell/personal-cabinet/provider/create-competition/create-judge/create-judge.component.ts
- src/app/shared/components/competition-card/competition-card.component.spec.ts
- src/app/shell/shell-routing.module.ts
- src/app/shared/services/competitions/user-competition.service.ts
- src/app/shared/utils/utils.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/app/shared/store/shared-user.actions.ts
[error] 86-86: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
src/app/shared/store/shared-user.state.ts
[error] 113-113: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/app/shell/personal-cabinet/provider/create-competition/create-judge/judge-form/judge-form.component.ts
[error] 27-27: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
src/app/shared/store/provider.state.ts
[error] 978-978: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 1010-1010: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 1036-1036: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/app/shell/details/competition-details/competition-details.component.spec.ts
[error] 54-61: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
🔇 Additional comments (18)
src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.ts (1)
79-84: Use.subscribe(...)instead of.forEach(...)for streams.
.forEach(...)on an Observable may only fire upon completion, which is unusual in this data flow. Consider using.subscribe(...)for clarity and to ensure that all emissions from the stream are handled.src/app/shared/services/images/images.service.ts (1)
24-31: Reintroduce null checks for better type safety.
This code does not handle cases whereentitymight be null or undefined, repeating a past concern aboutinstanceofchecks on possible serialized objects. Consider adding null checks and more robust type guarding.src/app/shared/store/shared-user.actions.ts (1)
16-19: LGTM! Well-structured action classes.The new competition-related actions follow the established patterns and naming conventions, making the codebase consistent and maintainable.
Also applies to: 31-34
src/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.ts (2)
34-49: LGTM! Well-structured form initialization.The form groups are well-organized with appropriate validators and the form group is correctly emitted to the parent component.
51-69: LGTM! Comprehensive geocoding result handling.The method handles both successful and failed geocoding scenarios, updates form values appropriately, and maintains form state.
src/app/shared/components/competition-card/competition-card.component.ts (2)
51-53: Track stub code for future removal.The stub code needs to be replaced when backend logic is implemented.
Would you like me to create an issue to track the removal of this stub code when backend integration is ready?
63-66: LGTM! Proper resource cleanup.The component correctly handles cleanup by completing and unsubscribing from observables in ngOnDestroy.
src/app/shell/details/details.component.ts (1)
95-103: LGTM! Well-structured entity handling.The getEntity method cleanly handles the addition of competition alongside existing workshop and provider logic.
src/app/shell/details/competition-details/competition-details.component.ts (2)
62-62: Remove hardcoded directionIds array.The directionIds are hardcoded to [1], which seems incorrect. This should be derived from the competition data.
65-65: Fix the method name to match the service.The pipeline failure indicates that the method name should be
getCarouselImagesinstead ofsetCarouselImages.src/app/shell/details/competition-details/competition-details.component.spec.ts (1)
63-65: Enhance test coverage.The test suite only covers component creation and dialog opening. Consider adding tests for:
- onTabChange method
- Competition initialization
- Error handling scenarios
src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.ts (1)
90-92: Add error handling to subscription.The subscription to competitions$ lacks error handling, which could lead to unhandled errors in production.
src/app/shell/personal-cabinet/provider/provider.module.ts (1)
39-39: Fix line length issue.The import statement exceeds the maximum line length of 140 characters.
src/app/shared/enum/modal-confirmation.ts (1)
5-5: LGTM!The new competition-related modal confirmation types follow the existing patterns and maintain consistency.
Also applies to: 48-49, 69-69, 103-104, 118-118, 152-153
src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.ts (2)
156-188: Enhance form validation.The form validation can be improved in several areas:
- Website, Facebook, and Instagram fields should validate URLs
- Registration date range should validate that end date is after start date
- Competition date range should validate that it's after registration date range
81-96: Add error handling for date operations.Date parsing and form operations should include error handling to prevent runtime errors.
src/app/shared/store/provider.actions.ts (1)
482-528: LGTM! Well-structured competition actions.The competition-related actions are well-organized, following consistent patterns and proper error handling.
src/app/shared/store/provider.state.ts (1)
1043-1051: Refresh competition list after deletion.The
deleteCompetitionByIdSuccessaction only shows a message but doesn't refresh the competition list.
| if (this.competition.areThereBenefits) { | ||
| this.benefitsOptionRadioBtn.setValue(this.competition.benefits, { emitEvent: false }); | ||
| this.DescriptionFormGroup.get('benefitsOptionsDesc').enable({ emitEvent: false }); | ||
| } |
There was a problem hiding this comment.
Potential mismatch between areThereBenefits and benefits.
You check this.competition.areThereBenefits but then call this.benefitsOptionRadioBtn.setValue(this.competition.benefits, { emitEvent: false });. This appears inconsistent and may cause logical errors if benefits is distinct or unset. Verify this logic and update the assigned property if necessary.
| filter(Boolean) | ||
| // return this code when added feature for published competition | ||
| // switchMap(() => { | ||
| // if (modalType === this.modalType.publishCompetition) { | ||
| // return this.store.dispatch(new PublishWorkshop(this.competition.organizerOfTheEventId)); | ||
| // } | ||
| // }) | ||
| ) | ||
| .subscribe(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to subscription.
The subscription lacks error handling, which could lead to unhandled errors in production.
dialogRef
.afterClosed()
.pipe(
filter(Boolean),
+ catchError(error => {
+ console.error('Error handling dialog action:', error);
+ return EMPTY;
+ })
)
.subscribe();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| filter(Boolean) | |
| // return this code when added feature for published competition | |
| // switchMap(() => { | |
| // if (modalType === this.modalType.publishCompetition) { | |
| // return this.store.dispatch(new PublishWorkshop(this.competition.organizerOfTheEventId)); | |
| // } | |
| // }) | |
| ) | |
| .subscribe(); | |
| } | |
| dialogRef | |
| .afterClosed() | |
| .pipe( | |
| filter(Boolean), | |
| catchError(error => { | |
| console.error('Error handling dialog action:', error); | |
| return EMPTY; | |
| }) | |
| ) | |
| .subscribe(); |
| beforeEach(async () => { | ||
| await TestBed.configureTestingModule({ | ||
| imports: [ | ||
| MatChipsModule, | ||
| MatTabsModule, | ||
| RouterTestingModule, | ||
| MatIconModule, | ||
| MatChipsModule, | ||
| NgxsModule.forRoot([]), | ||
| TranslateModule.forRoot(), | ||
| BrowserAnimationsModule, | ||
| MatDialogModule | ||
| ], | ||
| declarations: [ | ||
| CompetitionDetailsComponent, | ||
| MockCompetitionJudgesComponent, | ||
| MockCompetitionAboutComponent, | ||
| ImageCarouselComponent, | ||
| MockActionsComponent, | ||
| ConfirmationModalWindowComponent | ||
| ], | ||
| schemas: [NO_ERRORS_SCHEMA] | ||
| }).compileComponents(); | ||
| }); | ||
|
|
||
| beforeEach(() => { | ||
| fixture = TestBed.createComponent(CompetitionDetailsComponent); | ||
| component = fixture.componentInstance; | ||
| component.competition = {} as Competition; | ||
| component.provider = {} as Provider; | ||
| matDialog = TestBed.inject(MatDialog); | ||
| fixture.detectChanges(); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Merge duplicate beforeEach hooks.
The test suite has duplicate beforeEach hooks that can be merged for better maintainability.
beforeEach(async () => {
await TestBed.configureTestingModule({
imports: [
MatChipsModule,
MatTabsModule,
RouterTestingModule,
MatIconModule,
MatChipsModule,
NgxsModule.forRoot([]),
TranslateModule.forRoot(),
BrowserAnimationsModule,
MatDialogModule
],
declarations: [
CompetitionDetailsComponent,
MockCompetitionJudgesComponent,
MockCompetitionAboutComponent,
ImageCarouselComponent,
MockActionsComponent,
ConfirmationModalWindowComponent
],
schemas: [NO_ERRORS_SCHEMA]
}).compileComponents();
+ fixture = TestBed.createComponent(CompetitionDetailsComponent);
+ component = fixture.componentInstance;
+ component.competition = {} as Competition;
+ component.provider = {} as Provider;
+ matDialog = TestBed.inject(MatDialog);
+ fixture.detectChanges();
});
-
- beforeEach(() => {
- fixture = TestBed.createComponent(CompetitionDetailsComponent);
- component = fixture.componentInstance;
- component.competition = {} as Competition;
- component.provider = {} as Provider;
- matDialog = TestBed.inject(MatDialog);
- fixture.detectChanges();
- });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| beforeEach(async () => { | |
| await TestBed.configureTestingModule({ | |
| imports: [ | |
| MatChipsModule, | |
| MatTabsModule, | |
| RouterTestingModule, | |
| MatIconModule, | |
| MatChipsModule, | |
| NgxsModule.forRoot([]), | |
| TranslateModule.forRoot(), | |
| BrowserAnimationsModule, | |
| MatDialogModule | |
| ], | |
| declarations: [ | |
| CompetitionDetailsComponent, | |
| MockCompetitionJudgesComponent, | |
| MockCompetitionAboutComponent, | |
| ImageCarouselComponent, | |
| MockActionsComponent, | |
| ConfirmationModalWindowComponent | |
| ], | |
| schemas: [NO_ERRORS_SCHEMA] | |
| }).compileComponents(); | |
| }); | |
| beforeEach(() => { | |
| fixture = TestBed.createComponent(CompetitionDetailsComponent); | |
| component = fixture.componentInstance; | |
| component.competition = {} as Competition; | |
| component.provider = {} as Provider; | |
| matDialog = TestBed.inject(MatDialog); | |
| fixture.detectChanges(); | |
| }); | |
| beforeEach(async () => { | |
| await TestBed.configureTestingModule({ | |
| imports: [ | |
| MatChipsModule, | |
| MatTabsModule, | |
| RouterTestingModule, | |
| MatIconModule, | |
| MatChipsModule, | |
| NgxsModule.forRoot([]), | |
| TranslateModule.forRoot(), | |
| BrowserAnimationsModule, | |
| MatDialogModule | |
| ], | |
| declarations: [ | |
| CompetitionDetailsComponent, | |
| MockCompetitionJudgesComponent, | |
| MockCompetitionAboutComponent, | |
| ImageCarouselComponent, | |
| MockActionsComponent, | |
| ConfirmationModalWindowComponent | |
| ], | |
| schemas: [NO_ERRORS_SCHEMA] | |
| }).compileComponents(); | |
| fixture = TestBed.createComponent(CompetitionDetailsComponent); | |
| component = fixture.componentInstance; | |
| component.competition = {} as Competition; | |
| component.provider = {} as Provider; | |
| matDialog = TestBed.inject(MatDialog); | |
| fixture.detectChanges(); | |
| }); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 54-61: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
| const descInfo = this.DescriptionFormGroup.getRawValue(); | ||
| const address: Address = new Address(this.AddressFormGroup.value, this.competition?.address); | ||
| const judges: Judge[] = this.createJudges(); | ||
| console.log(judges); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove console.log statement.
Console.log statements should not be present in production code.
Apply this diff to remove the console.log:
- console.log(judges);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log(judges); |
| public RequiredFormGroup: FormGroup; | ||
| public DescriptionFormGroup: FormGroup; | ||
| public AddressFormGroup: FormGroup; | ||
| public JudgeFormArray: FormArray; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add form validation for required fields.
The form groups lack validation setup. Consider adding validators for required fields and showing validation errors to users.
Apply this diff to add form validation:
public RequiredFormGroup: FormGroup;
public DescriptionFormGroup: FormGroup;
public AddressFormGroup: FormGroup;
public JudgeFormArray: FormArray;
+
+ public get isFormValid(): boolean {
+ return (
+ this.RequiredFormGroup?.valid &&
+ this.DescriptionFormGroup?.valid &&
+ this.AddressFormGroup?.valid &&
+ (!this.JudgeFormArray?.length || this.JudgeFormArray?.valid)
+ );
+ }Then update the template to disable the submit button when the form is invalid:
- <button (click)="onSubmit()">Submit</button>
+ <button (click)="onSubmit()" [disabled]="!isFormValid">Submit</button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public RequiredFormGroup: FormGroup; | |
| public DescriptionFormGroup: FormGroup; | |
| public AddressFormGroup: FormGroup; | |
| public JudgeFormArray: FormArray; | |
| public RequiredFormGroup: FormGroup; | |
| public DescriptionFormGroup: FormGroup; | |
| public AddressFormGroup: FormGroup; | |
| public JudgeFormArray: FormArray; | |
| public get isFormValid(): boolean { | |
| return ( | |
| this.RequiredFormGroup?.valid && | |
| this.DescriptionFormGroup?.valid && | |
| this.AddressFormGroup?.valid && | |
| (!this.JudgeFormArray?.length || this.JudgeFormArray?.valid) | |
| ); | |
| } |
| public RequiredFormGroup: FormGroup; | |
| public DescriptionFormGroup: FormGroup; | |
| public AddressFormGroup: FormGroup; | |
| public JudgeFormArray: FormArray; | |
| <button (click)="onSubmit()" [disabled]="!isFormValid">Submit</button> |
| public onSubmit(): void { | ||
| const provider: Provider = this.store.selectSnapshot<Provider>(RegistrationState.provider); | ||
| const requiredInfo: CompetitionRequired = this.createRequired(); | ||
| const descInfo = this.DescriptionFormGroup.getRawValue(); | ||
| const address: Address = new Address(this.AddressFormGroup.value, this.competition?.address); | ||
| const judges: Judge[] = this.createJudges(); | ||
| console.log(judges); | ||
|
|
||
| let competition: Competition; | ||
|
|
||
| if (this.editMode) { | ||
| competition = new Competition(requiredInfo, descInfo, address, judges, provider, this.competition.id); | ||
| this.store.dispatch(new UpdateCompetition(competition)); | ||
| } else { | ||
| competition = new Competition(requiredInfo, descInfo, address, judges, provider); | ||
| this.store.dispatch(new CreateCompetition(competition)); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and loading state management.
The form submission lacks error handling and loading state management. Consider:
- Setting loading state during submission
- Handling potential errors from store actions
- Showing appropriate error messages to users
Apply this diff to add error handling:
public onSubmit(): void {
+ this.isLoading = true;
const provider: Provider = this.store.selectSnapshot<Provider>(RegistrationState.provider);
const requiredInfo: CompetitionRequired = this.createRequired();
const descInfo = this.DescriptionFormGroup.getRawValue();
const address: Address = new Address(this.AddressFormGroup.value, this.competition?.address);
const judges: Judge[] = this.createJudges();
let competition: Competition;
if (this.editMode) {
competition = new Competition(requiredInfo, descInfo, address, judges, provider, this.competition.id);
- this.store.dispatch(new UpdateCompetition(competition));
+ this.store.dispatch(new UpdateCompetition(competition))
+ .pipe(
+ finalize(() => this.isLoading = false)
+ )
+ .subscribe({
+ error: (error) => {
+ this.store.dispatch(new ShowMessageBar({ message: error.message, type: 'error' }));
+ }
+ });
} else {
competition = new Competition(requiredInfo, descInfo, address, judges, provider);
- this.store.dispatch(new CreateCompetition(competition));
+ this.store.dispatch(new CreateCompetition(competition))
+ .pipe(
+ finalize(() => this.isLoading = false)
+ )
+ .subscribe({
+ error: (error) => {
+ this.store.dispatch(new ShowMessageBar({ message: error.message, type: 'error' }));
+ }
+ });
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public onSubmit(): void { | |
| const provider: Provider = this.store.selectSnapshot<Provider>(RegistrationState.provider); | |
| const requiredInfo: CompetitionRequired = this.createRequired(); | |
| const descInfo = this.DescriptionFormGroup.getRawValue(); | |
| const address: Address = new Address(this.AddressFormGroup.value, this.competition?.address); | |
| const judges: Judge[] = this.createJudges(); | |
| console.log(judges); | |
| let competition: Competition; | |
| if (this.editMode) { | |
| competition = new Competition(requiredInfo, descInfo, address, judges, provider, this.competition.id); | |
| this.store.dispatch(new UpdateCompetition(competition)); | |
| } else { | |
| competition = new Competition(requiredInfo, descInfo, address, judges, provider); | |
| this.store.dispatch(new CreateCompetition(competition)); | |
| } | |
| } | |
| public onSubmit(): void { | |
| this.isLoading = true; | |
| const provider: Provider = this.store.selectSnapshot<Provider>(RegistrationState.provider); | |
| const requiredInfo: CompetitionRequired = this.createRequired(); | |
| const descInfo = this.DescriptionFormGroup.getRawValue(); | |
| const address: Address = new Address(this.AddressFormGroup.value, this.competition?.address); | |
| const judges: Judge[] = this.createJudges(); | |
| let competition: Competition; | |
| if (this.editMode) { | |
| competition = new Competition(requiredInfo, descInfo, address, judges, provider, this.competition.id); | |
| this.store.dispatch(new UpdateCompetition(competition)) | |
| .pipe( | |
| finalize(() => this.isLoading = false) | |
| ) | |
| .subscribe({ | |
| error: (error) => { | |
| this.store.dispatch(new ShowMessageBar({ message: error.message, type: 'error' })); | |
| } | |
| }); | |
| } else { | |
| competition = new Competition(requiredInfo, descInfo, address, judges, provider); | |
| this.store.dispatch(new CreateCompetition(competition)) | |
| .pipe( | |
| finalize(() => this.isLoading = false) | |
| ) | |
| .subscribe({ | |
| error: (error) => { | |
| this.store.dispatch(new ShowMessageBar({ message: error.message, type: 'error' })); | |
| } | |
| }); | |
| } | |
| } |
DimaShapoval
force-pushed
the
Romanchak/Create-competitive-event
branch
from
daeec8d to
45bbdc0
Compare
DimaShapoval
force-pushed
the
Romanchak/Create-competitive-event
branch
from
d706f6b to
fc013c1
Compare
There was a problem hiding this comment.
Actionable comments posted: 12
♻️ Duplicate comments (7)
src/app/shell/details/competition-details/competition-details.component.spec.ts (1)
29-61: 🛠️ Refactor suggestionMerge duplicate beforeEach hooks.
The test suite has duplicate beforeEach hooks that can be merged for better maintainability.
beforeEach(async () => { await TestBed.configureTestingModule({ imports: [ MatChipsModule, MatTabsModule, RouterTestingModule, MatIconModule, MatChipsModule, NgxsModule.forRoot([]), TranslateModule.forRoot(), BrowserAnimationsModule, MatDialogModule ], declarations: [ CompetitionDetailsComponent, MockCompetitionJudgesComponent, MockCompetitionAboutComponent, ImageCarouselComponent, MockActionsComponent, ConfirmationModalWindowComponent ], schemas: [NO_ERRORS_SCHEMA] }).compileComponents(); + fixture = TestBed.createComponent(CompetitionDetailsComponent); + component = fixture.componentInstance; + component.competition = {} as Competition; + component.provider = {} as Provider; + matDialog = TestBed.inject(MatDialog); + fixture.detectChanges(); }); - - beforeEach(() => { - fixture = TestBed.createComponent(CompetitionDetailsComponent); - component = fixture.componentInstance; - component.competition = {} as Competition; - component.provider = {} as Provider; - matDialog = TestBed.inject(MatDialog); - fixture.detectChanges(); - });🧰 Tools
🪛 Biome (1.9.4)
[error] 54-61: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.ts (1)
90-92: 🛠️ Refactor suggestionAdd error handling to subscription.
The subscription to competitions$ lacks error handling, which could lead to unhandled errors in production.
this.competitions$ - .pipe(takeUntil(this.destroy$)) + .pipe( + takeUntil(this.destroy$), + catchError(error => { + console.error('Error fetching competitions:', error); + return EMPTY; + }) + ) .subscribe((competitions: SearchResponse<CompetitionCardParameters[]>) => (this.competitions = competitions));src/app/shell/details/competition-details/competition-details.component.ts (2)
70-70:⚠️ Potential issueRemove hardcoded directionIds array.
The directionIds are hardcoded to [1], which seems incorrect. This should be derived from the competition data.
- this.competition.directionIds = [1];
84-96: 🛠️ Refactor suggestionAdd error handling to dialog subscription.
The dialog subscription lacks error handling, which could lead to unhandled errors in production.
dialogRef .afterClosed() .pipe( - filter(Boolean) + filter(Boolean), + catchError(error => { + console.error('Error handling dialog action:', error); + return EMPTY; + }) ) .subscribe();src/app/shell/personal-cabinet/provider/create-competition/create-judge/create-judge.component.ts (1)
42-52:⚠️ Potential issueUncomment and implement the essential functionality.
The commented-out code contains essential functionality for:
- Initializing judges in
ngOnInit- Creating and managing judge forms
- Handling edit mode
- Form validation and state management
The functionality needs to be implemented as discussed in past review comments.
Also applies to: 54-65, 67-97, 104-126, 132-134, 139-143, 145-155
src/app/shared/utils/utils.ts (1)
232-275: 🛠️ Refactor suggestionRefactor to eliminate code duplication with getWorkshopMessage.
The
getCompetitionMessagefunction duplicates the logic fromgetWorkshopMessage. Consider extracting the shared image handling logic into a reusable function.src/app/shared/store/provider.state.ts (1)
1043-1050: 🛠️ Refactor suggestionConsider refreshing the competition list after deletion.
The
deleteCompetitionByIdSuccessaction only shows a message but doesn't refresh the competition list. Consider dispatching aGetProviderViewCompetitionsaction to refresh the list.Apply this diff to refresh the list after deletion:
deleteCompetitionByIdSuccess( { dispatch }: StateContext<ProviderStateModel>, - { competition }: providerActions.DeleteCompetitionByIdSuccess + { competition, parameters }: providerActions.DeleteCompetitionByIdSuccess ): void { const messageData = Util.getCompetitionMessage(competition, SnackbarText.deleteCompetition); - dispatch([new MarkFormDirty(false), new ShowMessageBar({ message: messageData.message, type: messageData.type })]); + dispatch([ + new MarkFormDirty(false), + new ShowMessageBar({ message: messageData.message, type: messageData.type }), + new providerActions.GetProviderViewCompetitions(parameters) + ]); }
🧹 Nitpick comments (21)
src/app/shell/details/side-menu/side-menu.component.ts (1)
37-46: Consider creating a common interface for contact-related properties.The implementation looks good with proper null checking and fallback handling. However, since both
CompetitionandWorkshopshare similar contact-related properties, consider creating a common interface to improve type safety and maintainability.interface ContactsParent { contacts?: { phones?: { number: string }[]; emails?: { address: string }[]; socialNetworks?: { url: string }[]; address?: Address; }[]; }Then update the method signature:
-private getContactsData(contactsParent: Competition | Workshop): void { +private getContactsData(contactsParent: ContactsParent): void {src/app/shell/personal-cabinet/provider/create-competition/create-judge/judge-form/judge-form.component.html (1)
1-1: Verify custom CSS utility class usage.
The form element uses the classitems-betweenalong with common flex classes. Please confirm thatitems-betweenis an intended, valid CSS class in your project (or if it should be replaced with a standard utility class likejustify-between).src/app/shared/services/competitions/user-competition.service.ts (2)
22-28: Consider feature-based version selection for "getCompetitionById".Other methods (create/update) switch between v1/v2 based on the images feature. For consistency and maintainability, you might want to apply a similar logic here if the API is indeed versioned by features.
30-45: Align version usage with the rest of the service.This method consistently uses
/api/v1while other operations switch to/api/v2when the images feature is enabled. If you want a uniform approach, consider applying the same version-style check here as well.src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.ts (2)
37-41: Ensure naming consistency for form arrays.Angular style guidelines typically prefer camelCase for property names. Consider changing
ContactsFormArray,JudgeFormArray, etc. tocontactsFormArray,judgeFormArray, etc.
66-70: Avoid Boolean casting for route parameters.Currently, you cast parameter values to boolean, which can obscure the actual parameter values and cause confusion. Opt for a direct check of the actual string to make the code more readable.
- const id = Boolean(this.route.snapshot.paramMap.get('id')); - const param = Boolean(this.route.snapshot.paramMap.get('param')); - if (id && param) { - this.parentCompetition = this.route.snapshot.paramMap.get('id'); - } + const idParam = this.route.snapshot.paramMap.get('id'); + const secondParam = this.route.snapshot.paramMap.get('param'); + if (idParam && secondParam) { + this.parentCompetition = idParam; + }src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.ts (2)
112-114: Clarify the purpose ofsortTime()
ThesortTime()method currently returns 0 and has no effect. If it is a placeholder, remove it or implement its intended sorting behavior.
232-235: Improve clarity of seat control logic
Currently,setAvailableSeatsControlValue()conditionally enables/disables and sets seat values, which might be confusing to maintain. Consider extracting this logic to a well-named helper method or a small private function with descriptive naming to improve readability.src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.ts (1)
172-174: Clarify the purpose ofsortTime()
This function also returns a hard-coded0. If it's intended to sort or manipulate data, consider implementing its real logic or removing it as dead code.src/app/shell/personal-cabinet/provider/create-competition/create-judge/create-judge.component.spec.ts (1)
11-13: RefinemockMatDialogtype usage
mockMatDialogis typed as aMatDialogModulebut assigned a subset of functionality (onlyopen). For better clarity and type safety, consider using aMatDialogspy or Mock class instead.- let mockMatDialog: MatDialogModule; + let mockMatDialog: Partial<MatDialog>;src/app/shared/store/shared-user.actions.ts (1)
84-87: Remove unnecessary constructor.The empty constructor can be removed as it's not adding any functionality.
export class ResetProviderCompetitionDetails { static readonly type = '[user] clear Provider And Competition Details'; - constructor() {} }🧰 Tools
🪛 Biome (1.9.4)
[error] 86-86: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
src/app/shell/details/details.component.ts (1)
92-95: Update JSDoc comment to include Competition.The method comment is outdated and should be updated to include Competition.
/** - * This method get Workshop or Provider by Id; + * This method gets Workshop, Competition, or Provider by Id. */src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.ts (1)
100-102: Remove or document commented-out code.The commented-out code for employee workshops should either be removed if it's no longer needed or documented with a TODO comment if it's planned for future implementation.
- // else { - // this.store.dispatch(new GetEmployeeWorkshops(this.workshopCardParameters)); - // } + // TODO: Implement employee workshops feature in future iterationsrc/app/shell/details/competition-details/competition-details.component.ts (1)
88-94: Document commented-out code for future implementation.The commented-out code for publishing competition should be documented with a TODO comment.
- // return this code when added feature for published competition - // switchMap(() => { - // if (modalType === this.modalType.publishCompetition) { - // return this.store.dispatch(new PublishWorkshop(this.competition.organizerOfTheEventId)); - // } - // }) + // TODO: Implement competition publishing feature in future iteration + // Will use PublishWorkshop action with competition.organizerOfTheEventIdsrc/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.ts (4)
18-18: Remove commented code.The commented input property
@Input() public address: Address;should be removed if it's no longer needed.- // @Input() public address: Address;
33-33: UsetakeUntilDestroyedoperator instead of manual subscription management.Consider using Angular's
takeUntilDestroyedoperator to automatically handle subscription cleanup, reducing boilerplate code.- public destroy$: Subject<boolean> = new Subject<boolean>();Update the subscriptions to use
takeUntilDestroyed:- .pipe(takeUntil(this.destroy$)) + .pipe(takeUntilDestroyed())Remove the
ngOnDestroymethod:- public ngOnDestroy(): void { - this.destroy$.next(true); - this.destroy$.unsubscribe(); - }
89-96: Improve method documentation.The method documentation could be more descriptive about when and why the form needs to be marked as dirty.
/** - * This method makes addressFormGroup dirty + * Marks the address form group as dirty when the user interacts with the form. + * This ensures that validation states are properly reflected in the UI. */
191-191: Improve URL validation pattern.The current URL validation pattern for social networks could be more robust.
- url: new FormControl('', [Validators.pattern('^https?://[\\w\\d.-]+\\.[a-z]{2,}(?:/.*)?$')]) + url: new FormControl('', [Validators.pattern('^https?://(?:www\\.)?[\\w\\d.-]+\\.[a-z]{2,}(?:/[^\\s]*)?$')])This improved pattern:
- Makes
www.optional- Allows more characters in the path
- Prevents whitespace in URLs
src/app/shared/store/shared-user.state.ts (2)
113-113: Fix return type in method signature.The method's return type should be
Observable<Competition>instead ofObservable<Competition | void>since the error case is handled by the catchError operator.- ): Observable<Competition | void> { + ): Observable<Competition> {🧰 Tools
🪛 Biome (1.9.4)
[error] 113-113: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
149-150: Improve error handling message.The error message is too generic. Consider using a more specific error message from the competition context.
- dispatch(new ShowMessageBar({ message: SnackbarText.error, type: 'error' })); + dispatch(new ShowMessageBar({ message: SnackbarText.deletedCompetition || SnackbarText.error, type: 'error' }));src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.html (1)
1-9: Consider making image upload configuration more flexible.The image upload configuration could be improved by:
- Moving the image limit to a constant or configuration file
- Validating the cropper configuration
Apply this diff to improve configurability:
+ // In a constants file + export const MAX_COMPETITION_IMAGES = 10; + + // In the component - [imgMaxAmount]="10" + [imgMaxAmount]="MAX_COMPETITION_IMAGES"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/assets/i18n/en.jsonis excluded by!src/assets/**and included bysrc/**src/assets/i18n/uk.jsonis excluded by!src/assets/**and included bysrc/**
📒 Files selected for processing (90)
src/app/header/header.component.html(1 hunks)src/app/shared/components/competition-card/competition-card.component.html(1 hunks)src/app/shared/components/competition-card/competition-card.component.scss(1 hunks)src/app/shared/components/competition-card/competition-card.component.spec.ts(1 hunks)src/app/shared/components/competition-card/competition-card.component.ts(1 hunks)src/app/shared/components/sidenav-menu/sidenav-menu.component.html(1 hunks)src/app/shared/components/validation-hint/validation-hint.component.html(2 hunks)src/app/shared/components/validation-hint/validation-hint.component.ts(2 hunks)src/app/shared/components/workshop-card/workshop-card.component.ts(1 hunks)src/app/shared/constants/constants.ts(3 hunks)src/app/shared/constants/validation.ts(1 hunks)src/app/shared/enum/competition.ts(1 hunks)src/app/shared/enum/enumUA/competition.ts(1 hunks)src/app/shared/enum/enumUA/message-bar.ts(1 hunks)src/app/shared/enum/enumUA/navigation-bar.ts(1 hunks)src/app/shared/enum/enumUA/no-results.ts(1 hunks)src/app/shared/enum/modal-confirmation.ts(6 hunks)src/app/shared/models/address.model.ts(2 hunks)src/app/shared/models/codeficator.model.ts(1 hunks)src/app/shared/models/competition.model.ts(1 hunks)src/app/shared/models/institution.model.ts(1 hunks)src/app/shared/models/judge.model.ts(1 hunks)src/app/shared/models/provider.model.ts(1 hunks)src/app/shared/services/competitions/user-competition.service.spec.ts(1 hunks)src/app/shared/services/competitions/user-competition.service.ts(1 hunks)src/app/shared/services/images/images.service.ts(2 hunks)src/app/shared/shared.module.ts(3 hunks)src/app/shared/store/provider.actions.ts(3 hunks)src/app/shared/store/provider.state.ts(7 hunks)src/app/shared/store/shared-user.actions.ts(3 hunks)src/app/shared/store/shared-user.state.ts(7 hunks)src/app/shared/styles/create-form.scss(1 hunks)src/app/shared/utils/utils.ts(2 hunks)src/app/shell/details/competition-details/competition-details.component.html(1 hunks)src/app/shell/details/competition-details/competition-details.component.scss(1 hunks)src/app/shell/details/competition-details/competition-details.component.spec.ts(1 hunks)src/app/shell/details/competition-details/competition-details.component.ts(1 hunks)src/app/shell/details/details-tabs/competition-about/competition-about.component.html(1 hunks)src/app/shell/details/details-tabs/competition-about/competition-about.component.scss(1 hunks)src/app/shell/details/details-tabs/competition-about/competition-about.component.spec.ts(1 hunks)src/app/shell/details/details-tabs/competition-about/competition-about.component.ts(1 hunks)src/app/shell/details/details-tabs/competition-judges/competition-judges.component.html(1 hunks)src/app/shell/details/details-tabs/competition-judges/competition-judges.component.scss(1 hunks)src/app/shell/details/details-tabs/competition-judges/competition-judges.component.spec.ts(1 hunks)src/app/shell/details/details-tabs/competition-judges/competition-judges.component.ts(1 hunks)src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.html(1 hunks)src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.scss(1 hunks)src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.spec.ts(1 hunks)src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.ts(1 hunks)src/app/shell/details/details.component.html(2 hunks)src/app/shell/details/details.component.ts(5 hunks)src/app/shell/details/details.module.ts(2 hunks)src/app/shell/details/side-menu/side-menu.component.html(1 hunks)src/app/shell/details/side-menu/side-menu.component.ts(3 hunks)src/app/shell/personal-cabinet/personal-cabinet.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.scss(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.spec.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.scss(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.spec.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.scss(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.spec.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-judge/create-judge.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-judge/create-judge.component.scss(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-judge/create-judge.component.spec.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-judge/create-judge.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-judge/judge-form/judge-form.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-judge/judge-form/judge-form.component.scss(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-judge/judge-form/judge-form.component.spec.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-judge/judge-form/judge-form.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.scss(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.spec.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-workshop/create-about-form/create-about-form.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-workshop/create-workshop.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-workshop/create-workshop.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.html(1 hunks)src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.scss(1 hunks)src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.spec.ts(1 hunks)src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/provider-routing.module.ts(2 hunks)src/app/shell/personal-cabinet/provider/provider.module.ts(2 hunks)src/app/shell/personal-cabinet/shared-cabinet/applications/application-card/application-card.component.html(1 hunks)src/app/shell/shell-routing.module.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (61)
- src/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.scss
- src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.scss
- src/app/shared/components/sidenav-menu/sidenav-menu.component.html
- src/app/shell/personal-cabinet/provider/create-competition/create-judge/judge-form/judge-form.component.scss
- src/app/shared/models/institution.model.ts
- src/app/header/header.component.html
- src/app/shared/models/provider.model.ts
- src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.scss
- src/app/shell/details/details-tabs/competition-about/competition-about.component.ts
- src/app/shared/components/validation-hint/validation-hint.component.html
- src/app/shell/personal-cabinet/provider/provider-routing.module.ts
- src/app/shared/models/codeficator.model.ts
- src/app/shell/details/details-tabs/competition-judges/competition-judges.component.html
- src/app/shell/personal-cabinet/provider/create-workshop/create-about-form/create-about-form.component.ts
- src/app/shell/personal-cabinet/personal-cabinet.component.html
- src/app/shell/personal-cabinet/provider/create-workshop/create-workshop.component.ts
- src/app/shell/personal-cabinet/provider/create-competition/create-judge/create-judge.component.html
- src/app/shared/shared.module.ts
- src/app/shell/personal-cabinet/shared-cabinet/applications/application-card/application-card.component.html
- src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.spec.ts
- src/app/shell/details/competition-details/competition-details.component.scss
- src/app/shell/personal-cabinet/provider/create-competition/create-judge/create-judge.component.scss
- src/app/shared/components/validation-hint/validation-hint.component.ts
- src/app/shell/details/details-tabs/competition-judges/competition-judges.component.scss
- src/app/shell/details/details-tabs/competition-about/competition-about.component.spec.ts
- src/app/shared/models/address.model.ts
- src/app/shell/details/details.module.ts
- src/app/shared/services/competitions/user-competition.service.spec.ts
- src/app/shared/components/workshop-card/workshop-card.component.ts
- src/app/shared/enum/enumUA/competition.ts
- src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.html
- src/app/shell/personal-cabinet/provider/create-workshop/create-workshop.component.html
- src/app/shared/models/judge.model.ts
- src/app/shell/details/details.component.html
- src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.spec.ts
- src/app/shell/personal-cabinet/provider/provider-competition/provider-competition.component.html
- src/app/shared/constants/validation.ts
- src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.scss
- src/app/shared/enum/competition.ts
- src/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.html
- src/app/shell/details/details-tabs/competition-judges/competition-judges.component.spec.ts
- src/app/shared/components/competition-card/competition-card.component.scss
- src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.scss
- src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.html
- src/app/shared/components/competition-card/competition-card.component.html
- src/app/shared/enum/enumUA/message-bar.ts
- src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.scss
- src/app/shared/enum/enumUA/no-results.ts
- src/app/shell/details/details-tabs/competition-about/competition-about.component.scss
- src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.html
- src/app/shared/enum/enumUA/navigation-bar.ts
- src/app/shell/details/details-tabs/competition-about/competition-about.component.html
- src/app/shell/details/competition-details/competition-details.component.html
- src/app/shared/enum/modal-confirmation.ts
- src/app/shell/details/details-tabs/competition-judges/judge-card/judge-card.component.ts
- src/app/shell/details/details-tabs/competition-judges/competition-judges.component.ts
- src/app/shared/constants/constants.ts
- src/app/shell/personal-cabinet/provider/create-competition/create-judge/judge-form/judge-form.component.ts
- src/app/shared/components/competition-card/competition-card.component.spec.ts
- src/app/shared/styles/create-form.scss
- src/app/shared/components/competition-card/competition-card.component.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.spec.ts
[error] 31-40: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.spec.ts
[error] 68-96: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
src/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.spec.ts
[error] 39-44: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.spec.ts
[error] 63-84: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
src/app/shared/store/provider.state.ts
[error] 978-978: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 1010-1010: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 1036-1036: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/app/shared/store/shared-user.actions.ts
[error] 86-86: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
src/app/shared/store/shared-user.state.ts
[error] 113-113: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/app/shell/details/competition-details/competition-details.component.spec.ts
[error] 54-61: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
🪛 GitHub Check: build_and_test
src/app/shell/personal-cabinet/provider/provider.module.ts
[warning] 39-39:
This line has a length of 163. Maximum allowed is 140
🔇 Additional comments (23)
src/app/shell/details/side-menu/side-menu.component.html (1)
7-7: Clarify Conditional Rendering of<app-actions>The updated
*ngIfcondition now includes!competitionto ensure that the<app-actions>component is only rendered when there is no active competition. This change aligns with the new competitive event requirements, but please verify that this logic correctly reflects the intended behavior across different event types.src/app/shell/details/side-menu/side-menu.component.ts (2)
15-15: LGTM!The new input property follows the established pattern of other input properties in the component.
34-34: Verify the precedence order of workshop vs competition.The current implementation prioritizes workshop over competition (
this.workshop ?? this.competition). Please confirm if this is the intended behavior, as it means competition data will only be used when workshop is null or undefined.src/app/shared/services/competitions/user-competition.service.ts (2)
83-85: Use feature-based version selection for deleting competitions.A previous review noted this inconsistency. Either apply the same feature-based check (v1 vs. v2) here or explain why deletion must always target
/api/v2.
87-109: Enhance type safety increateFormData().The existing approach relies on string-based indexing (
Object.keys(competition)), which undermines TypeScript’s type checking. Consider casting the keys tokeyof Competitionto increase type safety.src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.ts (1)
110-129: Add error handling and loading state management.A previous review suggested adding error handling logic (e.g., showing messages on failure) and toggling a loading state while dispatching your store actions. Consider applying these suggestions to improve UX and resiliency.
src/app/shared/models/competition.model.ts (1)
69-73: Consolidate the duplicated assignments foroptionsForPeopleWithDisabilities.You set
optionsForPeopleWithDisabilitiestoBoolean(description.disabilityOptionsDesc)and then immediately overwrite it withdescription.optionsForPeopleWithDisabilities. Merge these into a single assignment to avoid confusion.src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.ts (2)
81-96: Consider adding competition date range validation
It appears thecompetitionDateRangeGroupdoesn't enforce that the end date must be strictly after the start date. A custom validator would ensure coherent scheduling for competitions.This was previously suggested. You may reuse the earlier approach of adding a custom date range validator to avoid overlapping or invalid intervals.
194-211: Verify the presence of valid provider data
TheuseProviderInfo()method assumesproviderfields exist. Ifprovideris null or partially loaded, callingthis.provider[ProviderWorkshopSameValues[value]]might result in undefined values or runtime errors.src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.ts (2)
82-87: Review the use of.forEach()on the observable
Using.forEach()oninstitutions$might behave unexpectedly if the observable emits multiple times. Prefer subscribing with.subscribe()(possibly combined withtake(1)orfirst()) to ensure the logic executes once and completes if that’s the intention.
214-217: Re-check the mismatch ofareThereBenefitsvs.benefits
WhenareThereBenefitsis true, the code sets the radio button value tocompetition.benefits. Confirm thatbenefitsis indeed the correct property to matchareThereBenefits.src/app/shared/services/images/images.service.ts (1)
20-22: Add null checks and improve type checking.The implementation still needs the improvements suggested in the previous review.
Additionally, consider extracting the hardcoded image paths to constants:
+ private readonly WORKSHOP_DEFAULT_IMAGE = 'assets/images/groupimages/workshop-img.png'; + private readonly COMPETITION_DEFAULT_IMAGE = 'assets/images/groupimages/competition-img.png'; + public getCarouselImages(entity: Workshop | Provider | Competition): ImgPath[] { let images: ImgPath[]; if (entity.imageIds?.length) { images = entity.imageIds.map((imgId: string) => ({ path: environment.storageUrl + imgId })); } else if (entity instanceof Workshop) { - images = [{ path: 'assets/images/groupimages/workshop-img.png' }]; + images = [{ path: this.WORKSHOP_DEFAULT_IMAGE }]; } else { - images = [{ path: 'assets/images/groupimages/competition-img.png' }]; + images = [{ path: this.COMPETITION_DEFAULT_IMAGE }]; }Also applies to: 24-35
src/app/shared/store/shared-user.actions.ts (1)
16-19: LGTM! Well-structured action classes.The competition-related actions follow the same pattern as workshop actions, maintaining consistency in the codebase.
Also applies to: 31-34
src/app/shell/details/details.component.ts (1)
33-34: LGTM! Well-structured competition integration.The competition-related changes follow the same patterns as workshop handling, maintaining consistency in the codebase. The component properly handles subscriptions and cleanup.
Also applies to: 41-41, 45-45, 60-60, 79-89, 98-99
src/app/shell/personal-cabinet/provider/provider.module.ts (1)
37-43: LGTM!The module declaration is properly structured with all necessary imports for the competition-related components.
The line length issue has already been addressed in past review comments.
🧰 Tools
🪛 GitHub Check: build_and_test
[warning] 39-39:
This line has a length of 163. Maximum allowed is 140src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.spec.ts (1)
31-84: LGTM!The test setup is properly structured with necessary module configuration and component initialization. The two
beforeEachhooks serve different purposes:
- First hook configures the testing module
- Second hook initializes the component and form group
🧰 Tools
🪛 Biome (1.9.4)
[error] 63-84: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
src/app/shell/personal-cabinet/provider/create-competition/create-competition-address/create-competition-address.component.spec.ts (3)
22-44: LGTM!The test setup is properly structured with necessary module configuration and component initialization. The two
beforeEachhooks serve different purposes:
- First hook configures the testing module
- Second hook initializes the component and form builder
🧰 Tools
🪛 Biome (1.9.4)
[error] 39-44: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
46-205: LGTM!The test suite is comprehensive and well-structured, covering:
- Component creation
- Form field management (phones, emails, socials)
- Address selection and validation
- Edit mode functionality
- Edge cases and error handling
208-224: LGTM!The mock components are properly defined with necessary inputs to support the test cases.
src/app/shell/shell-routing.module.ts (1)
183-196: LGTM! The competition routes are well-structured.The new routes for competition creation follow the established patterns and include the necessary guards for loading and deactivation.
src/app/shared/store/provider.actions.ts (2)
83-86: LGTM!The action class follows the established pattern and is well-structured.
482-528: LGTM!The action classes are well-structured with:
- Clear type strings for action identification
- Appropriate parameter types in constructors
- Proper pairing of success and failure handlers
src/app/shared/store/provider.state.ts (1)
336-349: LGTM!The method follows the established pattern with proper state management and error handling.
| @@ -1,4 +1,5 @@ | |||
| import { Component, Input, OnInit } from '@angular/core'; | |||
| import { Competition } from 'shared/models/competition.model'; | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the import path.
The import path starts with 'shared/' which might need to be prefixed with '../../../' to match the pattern used by other imports from the shared directory.
🏁 Script executed:
#!/bin/bash
# Description: Check if other files in the codebase use relative or absolute paths for shared imports
# Search for shared imports patterns
echo "Files using relative paths (../../../shared):"
rg "from '\.\./\.\./\.\./shared/" -l
echo -e "\nFiles using absolute paths (shared/):"
rg "from 'shared/" -lLength of output: 27010
Action Required: Consistent Relative Import Path
The file currently imports the Competition model using an absolute path:
import { Competition } from 'shared/models/competition.model';However, as seen in the codebase (e.g., in this file and others in the same folder), imports from the shared directory follow the relative path convention (using ../../../shared/...). To maintain consistency, please update the import statement to:
import { Competition } from '../../../shared/models/competition.model';| it('should create', () => { | ||
| expect(component).toBeTruthy(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add test coverage for key component features.
The current test suite only verifies component creation. Based on the component's functionality described in the AI summary, add test cases for:
- Form validation
- Chief judge selection handling
- Form modification events
Here's a template for additional test cases:
it('should initialize the judge form with required fields', () => {
expect(component.form.get('name')).toBeTruthy();
// Add expectations for other required fields
});
it('should emit form changes when modified', () => {
const emitSpy = spyOn(component.formChange, 'emit');
// Trigger form changes
expect(emitSpy).toHaveBeenCalled();
});
it('should handle chief judge selection correctly', () => {
// Test chief judge selection logic
});| import { ComponentFixture, TestBed } from '@angular/core/testing'; | ||
| import { TranslateModule } from '@ngx-translate/core'; | ||
| import { JudgeFormComponent } from './judge-form.component'; | ||
|
|
There was a problem hiding this comment.
Add missing form-related dependencies.
Since JudgeFormComponent uses reactive forms (as mentioned in the AI summary), the test module should include ReactiveFormsModule and related dependencies.
Add the following imports:
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { TranslateModule } from '@ngx-translate/core';
+import { ReactiveFormsModule } from '@angular/forms';
import { JudgeFormComponent } from './judge-form.component';And update the imports in the TestBed configuration:
TestBed.configureTestingModule({
declarations: [JudgeFormComponent],
- imports: [TranslateModule.forRoot()]
+ imports: [
+ TranslateModule.forRoot(),
+ ReactiveFormsModule
+ ]
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { ComponentFixture, TestBed } from '@angular/core/testing'; | |
| import { TranslateModule } from '@ngx-translate/core'; | |
| import { JudgeFormComponent } from './judge-form.component'; | |
| import { ComponentFixture, TestBed } from '@angular/core/testing'; | |
| import { TranslateModule } from '@ngx-translate/core'; | |
| import { ReactiveFormsModule } from '@angular/forms'; | |
| import { JudgeFormComponent } from './judge-form.component'; | |
| // ... other necessary imports or code ... | |
| describe('JudgeFormComponent', () => { | |
| let component: JudgeFormComponent; | |
| let fixture: ComponentFixture<JudgeFormComponent>; | |
| beforeEach(async () => { | |
| await TestBed.configureTestingModule({ | |
| declarations: [JudgeFormComponent], | |
| imports: [ | |
| TranslateModule.forRoot(), | |
| ReactiveFormsModule | |
| ] | |
| }) | |
| .compileComponents(); | |
| }); | |
| // ... additional test cases ... | |
| }); |
| beforeEach(() => { | ||
| TestBed.configureTestingModule({ | ||
| declarations: [JudgeFormComponent], | ||
| imports: [TranslateModule.forRoot()] | ||
| }); | ||
|
|
||
| fixture = TestBed.createComponent(JudgeFormComponent); | ||
| component = fixture.componentInstance; | ||
| }); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance test setup with form initialization and change detection.
The test setup should initialize the component's form and trigger change detection to ensure the component is in a stable state before testing.
Update the setup:
beforeEach(() => {
TestBed.configureTestingModule({
declarations: [JudgeFormComponent],
imports: [TranslateModule.forRoot()]
});
fixture = TestBed.createComponent(JudgeFormComponent);
component = fixture.componentInstance;
+ fixture.detectChanges();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| beforeEach(() => { | |
| TestBed.configureTestingModule({ | |
| declarations: [JudgeFormComponent], | |
| imports: [TranslateModule.forRoot()] | |
| }); | |
| fixture = TestBed.createComponent(JudgeFormComponent); | |
| component = fixture.componentInstance; | |
| }); | |
| beforeEach(() => { | |
| TestBed.configureTestingModule({ | |
| declarations: [JudgeFormComponent], | |
| imports: [TranslateModule.forRoot()] | |
| }); | |
| fixture = TestBed.createComponent(JudgeFormComponent); | |
| component = fixture.componentInstance; | |
| fixture.detectChanges(); | |
| }); |
| @@ -0,0 +1,101 @@ | |||
| <!-- <form [formGroup]="JudgeForm" class="flex flex-col justify-center items-between step"> | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure consistent naming for the reactive form group.
The form tag binds to [formGroup]="JudgeForm", yet all the validation hint components reference JudgeFormGroup. This inconsistency may lead to runtime errors. Please verify and use a single, consistent property name (for example, by renaming either one to match the other).
Also applies to: 10-10, 24-24, 41-41, 51-51, 67-67, 73-73, 89-89
| it('should create', () => { | ||
| expect(component).toBeTruthy(); | ||
| }); | ||
|
|
||
| it('should open confirmation dialog and dispatch PublishWorkshop on confirm', () => { | ||
| expectingMatDialogData = { | ||
| width: Constants.MODAL_SMALL, | ||
| data: { | ||
| type: ModalConfirmationType.publishCompetition | ||
| } | ||
| }; | ||
| matDialogSpy = jest.spyOn(matDialog, 'open').mockReturnValue({ | ||
| afterClosed: () => of(true) | ||
| } as MatDialogRef<ConfirmationModalWindowComponent>); | ||
| component.onActionButtonClick(ModalConfirmationType.publishCompetition); | ||
| expect(matDialogSpy).toHaveBeenCalledTimes(1); | ||
| expect(matDialogSpy).toHaveBeenCalledWith(ConfirmationModalWindowComponent, expectingMatDialogData); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance test coverage.
The test suite only covers component creation and dialog opening. Consider adding tests for:
- onTabChange method
- Competition initialization
- Error handling scenarios
Would you like me to help generate additional test cases to improve coverage?
| it('should set tags for competition and form field', fakeAsync(() => { | ||
| const mockCompetition: Partial<Competition> = { | ||
| competitiveSelection: true | ||
| }; | ||
| component.competition = mockCompetition as Competition; | ||
|
|
||
| component.ngOnInit(); | ||
| tick(); | ||
| })); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add assertions to the tag setting test.
The test for setting tags lacks assertions to verify the expected behavior.
it('should set tags for competition and form field', fakeAsync(() => {
const mockCompetition: Partial<Competition> = {
competitiveSelection: true
};
component.competition = mockCompetition as Competition;
component.ngOnInit();
tick();
+ expect(component.DescriptionFormGroup.get('competitiveSelection').value).toBe(true);
+ expect(component.DescriptionFormGroup.dirty).toBe(true);
}));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should set tags for competition and form field', fakeAsync(() => { | |
| const mockCompetition: Partial<Competition> = { | |
| competitiveSelection: true | |
| }; | |
| component.competition = mockCompetition as Competition; | |
| component.ngOnInit(); | |
| tick(); | |
| })); | |
| it('should set tags for competition and form field', fakeAsync(() => { | |
| const mockCompetition: Partial<Competition> = { | |
| competitiveSelection: true | |
| }; | |
| component.competition = mockCompetition as Competition; | |
| component.ngOnInit(); | |
| tick(); | |
| expect(component.DescriptionFormGroup.get('competitiveSelection').value).toBe(true); | |
| expect(component.DescriptionFormGroup.dirty).toBe(true); | |
| })); |
| beforeEach(async () => { | ||
| await TestBed.configureTestingModule({ | ||
| imports: [ | ||
| ReactiveFormsModule, | ||
| FormsModule, | ||
| HttpClientTestingModule, | ||
| BrowserAnimationsModule, | ||
| MaterialModule, | ||
| NgxsModule.forRoot([]), | ||
| TranslateModule.forRoot() | ||
| ], | ||
| declarations: [ | ||
| CreateCompetitionDescriptionFormComponent, | ||
| ImageFormControlComponent, | ||
| MockValidationHintAboutComponent, | ||
| MockInfoFormComponent | ||
| ], | ||
| providers: [ | ||
| { | ||
| provide: NG_VALUE_ACCESSOR, | ||
| // eslint-disable-next-line @angular-eslint/no-forward-ref | ||
| useExisting: forwardRef(() => ImageFormControlComponent), | ||
| multi: true | ||
| } | ||
| ] | ||
| }).compileComponents(); | ||
| }); | ||
|
|
||
| beforeEach(() => { | ||
| fixture = TestBed.createComponent(CreateCompetitionDescriptionFormComponent); | ||
| component = fixture.componentInstance; | ||
| component.provider = {} as any; | ||
| component.competition = { | ||
| competitiveSelection: false, | ||
| competitiveSelectionDescription: '', | ||
| keywords: [] | ||
| } as any; | ||
| component.DescriptionFormGroup = new FormGroup({ | ||
| imageFiles: new FormControl(''), | ||
| imageIds: new FormControl(['id1', 'id2', 'id3']), | ||
| description: new FormControl(''), | ||
| disabilityOptionsDesc: new FormControl(''), | ||
| formOfLearning: new FormControl(''), | ||
| competitiveSelection: new FormControl(''), | ||
| tagIds: new FormControl([]), | ||
| shortStay: new FormControl(false), | ||
| isSelfFinanced: new FormControl(false), | ||
| enrollmentProcedureDescription: new FormControl(''), | ||
| specialNeedsType: new FormControl('None'), | ||
| areThereBenefits: new FormControl(false), | ||
| preferentialTermsOfParticipation: new FormControl(''), | ||
| educationalShift: new FormControl('First'), | ||
| ageComposition: new FormControl('SameAge'), | ||
| coverage: new FormControl('School') | ||
| }); | ||
| fixture.detectChanges(); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Merge duplicate beforeEach hooks.
The test suite has duplicate beforeEach hooks that can be merged for better maintainability.
beforeEach(async () => {
await TestBed.configureTestingModule({
imports: [
ReactiveFormsModule,
FormsModule,
HttpClientTestingModule,
BrowserAnimationsModule,
MaterialModule,
NgxsModule.forRoot([]),
TranslateModule.forRoot()
],
declarations: [
CreateCompetitionDescriptionFormComponent,
ImageFormControlComponent,
MockValidationHintAboutComponent,
MockInfoFormComponent
],
providers: [
{
provide: NG_VALUE_ACCESSOR,
useExisting: forwardRef(() => ImageFormControlComponent),
multi: true
}
]
}).compileComponents();
+ fixture = TestBed.createComponent(CreateCompetitionDescriptionFormComponent);
+ component = fixture.componentInstance;
+ component.provider = {} as any;
+ component.competition = {
+ competitiveSelection: false,
+ competitiveSelectionDescription: '',
+ keywords: []
+ } as any;
+ component.DescriptionFormGroup = new FormGroup({
+ imageFiles: new FormControl(''),
+ imageIds: new FormControl(['id1', 'id2', 'id3']),
+ description: new FormControl(''),
+ disabilityOptionsDesc: new FormControl(''),
+ formOfLearning: new FormControl(''),
+ competitiveSelection: new FormControl(''),
+ tagIds: new FormControl([]),
+ shortStay: new FormControl(false),
+ isSelfFinanced: new FormControl(false),
+ enrollmentProcedureDescription: new FormControl(''),
+ specialNeedsType: new FormControl('None'),
+ areThereBenefits: new FormControl(false),
+ preferentialTermsOfParticipation: new FormControl(''),
+ educationalShift: new FormControl('First'),
+ ageComposition: new FormControl('SameAge'),
+ coverage: new FormControl('School')
+ });
+ fixture.detectChanges();
});
-
- beforeEach(() => {
- fixture = TestBed.createComponent(CreateCompetitionDescriptionFormComponent);
- component = fixture.componentInstance;
- component.provider = {} as any;
- component.competition = {
- competitiveSelection: false,
- competitiveSelectionDescription: '',
- keywords: []
- } as any;
- component.DescriptionFormGroup = new FormGroup({
- imageFiles: new FormControl(''),
- imageIds: new FormControl(['id1', 'id2', 'id3']),
- description: new FormControl(''),
- disabilityOptionsDesc: new FormControl(''),
- formOfLearning: new FormControl(''),
- competitiveSelection: new FormControl(''),
- tagIds: new FormControl([]),
- shortStay: new FormControl(false),
- isSelfFinanced: new FormControl(false),
- enrollmentProcedureDescription: new FormControl(''),
- specialNeedsType: new FormControl('None'),
- areThereBenefits: new FormControl(false),
- preferentialTermsOfParticipation: new FormControl(''),
- educationalShift: new FormControl('First'),
- ageComposition: new FormControl('SameAge'),
- coverage: new FormControl('School')
- });
- fixture.detectChanges();
- });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| beforeEach(async () => { | |
| await TestBed.configureTestingModule({ | |
| imports: [ | |
| ReactiveFormsModule, | |
| FormsModule, | |
| HttpClientTestingModule, | |
| BrowserAnimationsModule, | |
| MaterialModule, | |
| NgxsModule.forRoot([]), | |
| TranslateModule.forRoot() | |
| ], | |
| declarations: [ | |
| CreateCompetitionDescriptionFormComponent, | |
| ImageFormControlComponent, | |
| MockValidationHintAboutComponent, | |
| MockInfoFormComponent | |
| ], | |
| providers: [ | |
| { | |
| provide: NG_VALUE_ACCESSOR, | |
| // eslint-disable-next-line @angular-eslint/no-forward-ref | |
| useExisting: forwardRef(() => ImageFormControlComponent), | |
| multi: true | |
| } | |
| ] | |
| }).compileComponents(); | |
| }); | |
| beforeEach(() => { | |
| fixture = TestBed.createComponent(CreateCompetitionDescriptionFormComponent); | |
| component = fixture.componentInstance; | |
| component.provider = {} as any; | |
| component.competition = { | |
| competitiveSelection: false, | |
| competitiveSelectionDescription: '', | |
| keywords: [] | |
| } as any; | |
| component.DescriptionFormGroup = new FormGroup({ | |
| imageFiles: new FormControl(''), | |
| imageIds: new FormControl(['id1', 'id2', 'id3']), | |
| description: new FormControl(''), | |
| disabilityOptionsDesc: new FormControl(''), | |
| formOfLearning: new FormControl(''), | |
| competitiveSelection: new FormControl(''), | |
| tagIds: new FormControl([]), | |
| shortStay: new FormControl(false), | |
| isSelfFinanced: new FormControl(false), | |
| enrollmentProcedureDescription: new FormControl(''), | |
| specialNeedsType: new FormControl('None'), | |
| areThereBenefits: new FormControl(false), | |
| preferentialTermsOfParticipation: new FormControl(''), | |
| educationalShift: new FormControl('First'), | |
| ageComposition: new FormControl('SameAge'), | |
| coverage: new FormControl('School') | |
| }); | |
| fixture.detectChanges(); | |
| }); | |
| beforeEach(async () => { | |
| await TestBed.configureTestingModule({ | |
| imports: [ | |
| ReactiveFormsModule, | |
| FormsModule, | |
| HttpClientTestingModule, | |
| BrowserAnimationsModule, | |
| MaterialModule, | |
| NgxsModule.forRoot([]), | |
| TranslateModule.forRoot() | |
| ], | |
| declarations: [ | |
| CreateCompetitionDescriptionFormComponent, | |
| ImageFormControlComponent, | |
| MockValidationHintAboutComponent, | |
| MockInfoFormComponent | |
| ], | |
| providers: [ | |
| { | |
| provide: NG_VALUE_ACCESSOR, | |
| // eslint-disable-next-line @angular-eslint/no-forward-ref | |
| useExisting: forwardRef(() => ImageFormControlComponent), | |
| multi: true | |
| } | |
| ] | |
| }).compileComponents(); | |
| fixture = TestBed.createComponent(CreateCompetitionDescriptionFormComponent); | |
| component = fixture.componentInstance; | |
| component.provider = {} as any; | |
| component.competition = { | |
| competitiveSelection: false, | |
| competitiveSelectionDescription: '', | |
| keywords: [] | |
| } as any; | |
| component.DescriptionFormGroup = new FormGroup({ | |
| imageFiles: new FormControl(''), | |
| imageIds: new FormControl(['id1', 'id2', 'id3']), | |
| description: new FormControl(''), | |
| disabilityOptionsDesc: new FormControl(''), | |
| formOfLearning: new FormControl(''), | |
| competitiveSelection: new FormControl(''), | |
| tagIds: new FormControl([]), | |
| shortStay: new FormControl(false), | |
| isSelfFinanced: new FormControl(false), | |
| enrollmentProcedureDescription: new FormControl(''), | |
| specialNeedsType: new FormControl('None'), | |
| areThereBenefits: new FormControl(false), | |
| preferentialTermsOfParticipation: new FormControl(''), | |
| educationalShift: new FormControl('First'), | |
| ageComposition: new FormControl('SameAge'), | |
| coverage: new FormControl('School') | |
| }); | |
| fixture.detectChanges(); | |
| }); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 68-96: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
| <label class="step-label"> {{ 'FORMS.LABELS.SUPPORT_PERSONS_WITH_DISABILITIES' | translate }} </label> | ||
| <mat-radio-group [formControl]="disabilityOptionRadioBtn" color="primary" class="flex flex-col justify-center items-between"> | ||
| <mat-radio-button #radio1 name="radioBtn1" [value]="false">{{ 'NOT_AVAILABLE' | translate }}</mat-radio-button> | ||
| <mat-radio-button #radio2 name="radioBtn2" [value]="true">{{ 'AVAILABLE' | translate }}</mat-radio-button> | ||
| <input | ||
| matInput | ||
| class="step-input" | ||
| [ngClass]="{ 'disabled-field': !disabilityOptionRadioBtn.value }" | ||
| placeholder="{{ 'FORMS.PLACEHOLDERS.SPECIAL_EQUIPMENT' | translate }}" | ||
| formControlName="disabilityOptionsDesc" | ||
| autocomplete="off" | ||
| appTrimValue /> | ||
| <app-validation-hint | ||
| [validationFormControl]="DescriptionFormGroup.get('disabilityOptionsDesc')" | ||
| [minCharacters]="validationConstants.INPUT_LENGTH_1" | ||
| [maxCharacters]="validationConstants.INPUT_LENGTH_256"> | ||
| </app-validation-hint> | ||
| </mat-radio-group> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve UX for disability options section.
The disability options section needs better UX:
- The description input should be properly disabled when "Not Available" is selected
- Validation hints should only be shown when the input is enabled
Apply this diff to improve the UX:
<input
matInput
class="step-input"
[ngClass]="{ 'disabled-field': !disabilityOptionRadioBtn.value }"
+ [disabled]="!disabilityOptionRadioBtn.value"
placeholder="{{ 'FORMS.PLACEHOLDERS.SPECIAL_EQUIPMENT' | translate }}"
formControlName="disabilityOptionsDesc"
autocomplete="off"
appTrimValue />
<app-validation-hint
+ *ngIf="disabilityOptionRadioBtn.value"
[validationFormControl]="DescriptionFormGroup.get('disabilityOptionsDesc')"
[minCharacters]="validationConstants.INPUT_LENGTH_1"
[maxCharacters]="validationConstants.INPUT_LENGTH_256">
</app-validation-hint>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <label class="step-label"> {{ 'FORMS.LABELS.SUPPORT_PERSONS_WITH_DISABILITIES' | translate }} </label> | |
| <mat-radio-group [formControl]="disabilityOptionRadioBtn" color="primary" class="flex flex-col justify-center items-between"> | |
| <mat-radio-button #radio1 name="radioBtn1" [value]="false">{{ 'NOT_AVAILABLE' | translate }}</mat-radio-button> | |
| <mat-radio-button #radio2 name="radioBtn2" [value]="true">{{ 'AVAILABLE' | translate }}</mat-radio-button> | |
| <input | |
| matInput | |
| class="step-input" | |
| [ngClass]="{ 'disabled-field': !disabilityOptionRadioBtn.value }" | |
| placeholder="{{ 'FORMS.PLACEHOLDERS.SPECIAL_EQUIPMENT' | translate }}" | |
| formControlName="disabilityOptionsDesc" | |
| autocomplete="off" | |
| appTrimValue /> | |
| <app-validation-hint | |
| [validationFormControl]="DescriptionFormGroup.get('disabilityOptionsDesc')" | |
| [minCharacters]="validationConstants.INPUT_LENGTH_1" | |
| [maxCharacters]="validationConstants.INPUT_LENGTH_256"> | |
| </app-validation-hint> | |
| </mat-radio-group> | |
| <label class="step-label"> {{ 'FORMS.LABELS.SUPPORT_PERSONS_WITH_DISABILITIES' | translate }} </label> | |
| <mat-radio-group [formControl]="disabilityOptionRadioBtn" color="primary" class="flex flex-col justify-center items-between"> | |
| <mat-radio-button #radio1 name="radioBtn1" [value]="false">{{ 'NOT_AVAILABLE' | translate }}</mat-radio-button> | |
| <mat-radio-button #radio2 name="radioBtn2" [value]="true">{{ 'AVAILABLE' | translate }}</mat-radio-button> | |
| <input | |
| matInput | |
| class="step-input" | |
| [ngClass]="{ 'disabled-field': !disabilityOptionRadioBtn.value }" | |
| [disabled]="!disabilityOptionRadioBtn.value" | |
| placeholder="{{ 'FORMS.PLACEHOLDERS.SPECIAL_EQUIPMENT' | translate }}" | |
| formControlName="disabilityOptionsDesc" | |
| autocomplete="off" | |
| appTrimValue /> | |
| <app-validation-hint | |
| *ngIf="disabilityOptionRadioBtn.value" | |
| [validationFormControl]="DescriptionFormGroup.get('disabilityOptionsDesc')" | |
| [minCharacters]="validationConstants.INPUT_LENGTH_1" | |
| [maxCharacters]="validationConstants.INPUT_LENGTH_256"> | |
| </app-validation-hint> | |
| </mat-radio-group> |
| <label class="step-label">{{ 'FORMS.LABELS.PRICE' | translate }}</label> | ||
| <mat-radio-group [formControl]="priceRadioBtn" color="primary"> | ||
| <mat-radio-button #priceRadio1 name="priceRadioBtn1" [value]="false">{{ 'FOR_FREE' | translate }}</mat-radio-button> | ||
| <div class="container flex flex-row justify-between items-center"> | ||
| <div class="price flex flex-row justify-center items-center"> | ||
| <mat-radio-button #priceRadio2 name="priceRadioBtn2" [value]="true" | ||
| ><input | ||
| matInput | ||
| class="step-input price-input" | ||
| appDigitOnly | ||
| appMinMax | ||
| formControlName="price" | ||
| type="number" | ||
| [min]="validationConstants.MIN_PRICE" | ||
| [max]="validationConstants.MAX_PRICE" /> | ||
| </mat-radio-button> | ||
| <p class="step-text">{{ 'FORMS.UAH' | translate }}</p> | ||
| </div> | ||
| </div> | ||
| </mat-radio-group> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve accessibility of the price input section.
The price input section needs better accessibility support:
- Add aria labels for screen readers
- Associate the currency text with the input using aria-describedby
Apply this diff to improve accessibility:
<input
matInput
class="step-input price-input"
+ [attr.aria-label]="'FORMS.LABELS.PRICE' | translate"
appDigitOnly
appMinMax
formControlName="price"
type="number"
[min]="validationConstants.MIN_PRICE"
- [max]="validationConstants.MAX_PRICE" />
+ [max]="validationConstants.MAX_PRICE"
+ [attr.aria-describedby]="'currency-text'" />
- <p class="step-text">{{ 'FORMS.UAH' | translate }}</p>
+ <p id="currency-text" class="step-text">{{ 'FORMS.UAH' | translate }}</p>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <label class="step-label">{{ 'FORMS.LABELS.PRICE' | translate }}</label> | |
| <mat-radio-group [formControl]="priceRadioBtn" color="primary"> | |
| <mat-radio-button #priceRadio1 name="priceRadioBtn1" [value]="false">{{ 'FOR_FREE' | translate }}</mat-radio-button> | |
| <div class="container flex flex-row justify-between items-center"> | |
| <div class="price flex flex-row justify-center items-center"> | |
| <mat-radio-button #priceRadio2 name="priceRadioBtn2" [value]="true" | |
| ><input | |
| matInput | |
| class="step-input price-input" | |
| appDigitOnly | |
| appMinMax | |
| formControlName="price" | |
| type="number" | |
| [min]="validationConstants.MIN_PRICE" | |
| [max]="validationConstants.MAX_PRICE" /> | |
| </mat-radio-button> | |
| <p class="step-text">{{ 'FORMS.UAH' | translate }}</p> | |
| </div> | |
| </div> | |
| </mat-radio-group> | |
| <label class="step-label">{{ 'FORMS.LABELS.PRICE' | translate }}</label> | |
| <mat-radio-group [formControl]="priceRadioBtn" color="primary"> | |
| <mat-radio-button #priceRadio1 name="priceRadioBtn1" [value]="false">{{ 'FOR_FREE' | translate }}</mat-radio-button> | |
| <div class="container flex flex-row justify-between items-center"> | |
| <div class="price flex flex-row justify-center items-center"> | |
| <mat-radio-button #priceRadio2 name="priceRadioBtn2" [value]="true" | |
| ><input | |
| matInput | |
| class="step-input price-input" | |
| [attr.aria-label]="'FORMS.LABELS.PRICE' | translate" | |
| appDigitOnly | |
| appMinMax | |
| formControlName="price" | |
| type="number" | |
| [min]="validationConstants.MIN_PRICE" | |
| [max]="validationConstants.MAX_PRICE" | |
| [attr.aria-describedby]="'currency-text'" /> | |
| </mat-radio-button> | |
| <p id="currency-text" class="step-text">{{ 'FORMS.UAH' | translate }}</p> | |
| </div> | |
| </div> | |
| </mat-radio-group> |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
src/app/shared/models/competition.model.ts (4)
53-53: Follow TypeScript naming conventions for parametersThe parameter
Contactsuses PascalCase, which is inconsistent with TypeScript conventions. Parameters should use camelCase.constructor( required: CompetitionRequired, description: Description, - Contacts: CompetitionContacts[], + contacts: CompetitionContacts[], judges: Judge[], provider: Provider, id?: string ) {Don't forget to update the reference at line 66 as well:
- this.contacts = Contacts; + this.contacts = contacts;
225-225: Avoid recursive type definitionThe
CompetitionContactsconstructor parameter uses the same class type, creating a circular reference. Consider creating a separate interface for the constructor parameter.- constructor(info: CompetitionContacts) { + constructor(info: { + title: string; + isDefault: boolean; + address: Address; + phones: CompetitionPhone[]; + emails: CompetitionEmail[]; + socialNetworks?: CompetitionSocialNetwork[]; + }) {
238-251: Consider making the Description interface exportedThe
Descriptioninterface is used as a parameter in the public constructor ofCompetitionBase, but it's not exported. This could make it difficult for consumers of this class to create valid objects.- interface Description { + export interface Description {
253-266: Export communication interfaces for reusabilityThe interfaces
CompetitionPhone,CompetitionEmail, andCompetitionSocialNetworkare not exported, limiting their reusability in other parts of the codebase that might need to work with competition contact information.- interface CompetitionPhone { + export interface CompetitionPhone {Apply similar changes to the other interfaces.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/shared/enum/competition.ts(1 hunks)src/app/shared/models/competition.model.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/shared/enum/competition.ts
🔇 Additional comments (2)
src/app/shared/models/competition.model.ts (2)
69-72: Fix the overwritten property assignmentLine 69 sets
optionsForPeopleWithDisabilitiesto a boolean conversion ofdescription.disabilityOptionsDesc, but line 72 immediately overwrites it withdescription.optionsForPeopleWithDisabilities. This duplicated assignment causes confusion and potential bugs.Apply this change to fix the issue:
- this.optionsForPeopleWithDisabilities = Boolean(description.disabilityOptionsDesc); this.competitiveSelection = Boolean(description.descriptionOfTheEnrollmentProcedure); this.areThereBenefits = Boolean(description.benefitsOptionsDesc); - this.optionsForPeopleWithDisabilities = description.optionsForPeopleWithDisabilities; + this.optionsForPeopleWithDisabilities = description.optionsForPeopleWithDisabilities ?? Boolean(description.disabilityOptionsDesc);
9-49: Initialize all declared properties in the constructorSeveral properties in
CompetitionBaseare declared but never initialized in the constructor:
childParticipanttermsOfParticipationpreferentialTermsOfParticipationdirectionIdsparentIdbuildingHoldingIdchildParticipantIdparticipantsOfTheEventThis can lead to incomplete objects and potential runtime errors.
Please ensure all properties are properly initialized with appropriate values or defaults.
| enum Socials { | ||
| Facebook = 'Facebook', | ||
| Instagram = 'Instagram', | ||
| Website = 'Website' | ||
| } |
There was a problem hiding this comment.
Export the Socials enum
The Socials enum is used in the public interface CompetitionSocialNetwork but isn't exported. This could cause compilation errors when other parts of the codebase try to use this interface.
- enum Socials {
+ export enum Socials {
Facebook = 'Facebook',
Instagram = 'Instagram',
Website = 'Website'
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| enum Socials { | |
| Facebook = 'Facebook', | |
| Instagram = 'Instagram', | |
| Website = 'Website' | |
| } | |
| export enum Socials { | |
| Facebook = 'Facebook', | |
| Instagram = 'Instagram', | |
| Website = 'Website' | |
| } |
| if (required.registrationDateRangeGroup.start) { | ||
| this.registrationStartTime = new Date(required.registrationDateRangeGroup.start).toISOString(); | ||
| } | ||
| if (required.registrationDateRangeGroup.end) { | ||
| this.registrationEndTime = new Date(required.registrationDateRangeGroup.end).toISOString(); | ||
| } |
There was a problem hiding this comment.
Add null check for optional registrationDateRangeGroup
The registrationDateRangeGroup is defined as optional in the CompetitionRequired interface, but it's accessed directly without checking if it exists. This could lead to runtime errors if the property is undefined.
Apply this change:
- if (required.registrationDateRangeGroup.start) {
+ if (required.registrationDateRangeGroup?.start) {
this.registrationStartTime = new Date(required.registrationDateRangeGroup.start).toISOString();
}
- if (required.registrationDateRangeGroup.end) {
+ if (required.registrationDateRangeGroup?.end) {
this.registrationEndTime = new Date(required.registrationDateRangeGroup.end).toISOString();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (required.registrationDateRangeGroup.start) { | |
| this.registrationStartTime = new Date(required.registrationDateRangeGroup.start).toISOString(); | |
| } | |
| if (required.registrationDateRangeGroup.end) { | |
| this.registrationEndTime = new Date(required.registrationDateRangeGroup.end).toISOString(); | |
| } | |
| if (required.registrationDateRangeGroup?.start) { | |
| this.registrationStartTime = new Date(required.registrationDateRangeGroup.start).toISOString(); | |
| } | |
| if (required.registrationDateRangeGroup?.end) { | |
| this.registrationEndTime = new Date(required.registrationDateRangeGroup.end).toISOString(); | |
| } |
| competitiveEventAccountingTypeId: number; | ||
| parentCompetition?: string; | ||
| numberOfSeats: number; | ||
| coverImageId?: string; |
There was a problem hiding this comment.
Inconsistent type for coverImageId
In the CompetitionRequired interface, coverImageId is defined as a string, but in the Competition constructor, it's treated as an array when assigning this.coverImageId = required.coverImageId[0].
The type should be consistent. If it should be an array of strings, update the interface:
- coverImageId?: string;
+ coverImageId?: string[] | string;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| coverImageId?: string; | |
| coverImageId?: string[] | string; |
| constructor( | ||
| required: CompetitionRequired, | ||
| description: Description, | ||
| contacts: CompetitionContacts[], | ||
| judges: Judge[], | ||
| provider: Provider, | ||
| id?: string | ||
| ) { | ||
| super(required, description, contacts, judges, provider, id); | ||
|
|
||
| if (required.coverImageId) { | ||
| this.coverImageId = required.coverImageId[0]; | ||
| } | ||
| if (required.coverImage) { | ||
| this.coverImage = required.coverImage; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Consider adding validation for properties in Competition constructor
The constructor sets coverImageId from required.coverImageId[0] without checking if coverImageId is an array or if it has elements. This could lead to runtime errors.
Apply this change:
if (required.coverImageId) {
- this.coverImageId = required.coverImageId[0];
+ this.coverImageId = Array.isArray(required.coverImageId) && required.coverImageId.length > 0
+ ? required.coverImageId[0]
+ : required.coverImageId.toString();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| constructor( | |
| required: CompetitionRequired, | |
| description: Description, | |
| contacts: CompetitionContacts[], | |
| judges: Judge[], | |
| provider: Provider, | |
| id?: string | |
| ) { | |
| super(required, description, contacts, judges, provider, id); | |
| if (required.coverImageId) { | |
| this.coverImageId = required.coverImageId[0]; | |
| } | |
| if (required.coverImage) { | |
| this.coverImage = required.coverImage; | |
| } | |
| } | |
| } | |
| constructor( | |
| required: CompetitionRequired, | |
| description: Description, | |
| contacts: CompetitionContacts[], | |
| judges: Judge[], | |
| provider: Provider, | |
| id?: string | |
| ) { | |
| super(required, description, contacts, judges, provider, id); | |
| if (required.coverImageId) { | |
| this.coverImageId = Array.isArray(required.coverImageId) && required.coverImageId.length > 0 | |
| ? required.coverImageId[0] | |
| : required.coverImageId.toString(); | |
| } | |
| if (required.coverImage) { | |
| this.coverImage = required.coverImage; | |
| } | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (14)
src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.html (3)
1-9: Consider adding descriptive ARIA labels for screen readers.
For the uploaded cover image section, adding ARIA labels or descriptive text can improve accessibility for users relying on screen readers.
85-111: Improve date range accessibility.
Providing an explicitaria-labelor screen-reader-friendly text for both start and end date inputs can improve accessibility for visually impaired users.
128-159: Remove duplicate class attributes on containing div.
At lines 137–138,<div class="container" class="flex ...">is repeated. This can lead to confusion or override.- <div class="container" class="flex flex-row justify-between items-center"> + <div class="container flex flex-row justify-between items-center">src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.html (2)
1-29: Leverage a shared or nested form approach for repeated patterns.
Multiple sections (image forms, information forms) could be more maintainable if refactored into separate reusable components. This reduces code duplication.
105-122: Hide validation hint when input is disabled.
If “Not Available” is selected, the input is disabled but the validation hint is still shown. Consider conditionally hiding or showing the hint to reduce confusion for users.src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.ts (2)
66-71: Clarify boolean checks of route params.
UsingBoolean(this.route.snapshot.paramMap.get('id'))andBoolean(this.route.snapshot.paramMap.get('param'))can be confusing. You might explicitly check if param or id is not null or undefined for readability and maintainability.
113-129: Consider adding error handling and loading states.
TheonSubmit()logic dispatches store actions without any catch for errors or indication of a loading state. Incorporate error handling and loading indicators for better UX.src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.ts (1)
83-88: Usesubscribeinstead offorEachon Observables for clarity.
this.institutions$.forEach(...)is a bit unusual. Typically,.subscribe(...)or.pipe(...)is used for Observables to avoid confusion with arrays.src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.ts (6)
33-42: Consider adding image dimension validation.The cropperConfig looks well-defined, but there's no validation to ensure that uploaded images meet these dimension requirements before they reach the cropper.
Consider adding validation to check image dimensions when files are selected:
public validateImageDimensions(file: File): Promise<boolean> { return new Promise((resolve) => { const img = new Image(); img.onload = () => { const isValid = img.width >= this.cropperConfig.cropperMinWidth && img.width <= this.cropperConfig.cropperMaxWidth && img.height >= this.cropperConfig.cropperMinHeight && img.height <= this.cropperConfig.cropperMaxHeight; resolve(isValid); }; img.src = URL.createObjectURL(file); }); }
112-114: Remove or document the purpose of the sortTime method.The method simply returns 0 without apparent usage. Either document its purpose or remove it if unused.
If the method is meant to be overridden in child classes, add a comment explaining this:
/** * Method intended to be overridden by child classes to provide custom sorting logic * @returns Default sort value of 0 */ public sortTime(): number { return 0; }
119-159: Complex date handling could be refactored.The activateEditMode method contains complex nested date handling which could be refactored into separate methods for better maintainability.
Consider refactoring like this:
public activateEditMode(): void { this.RequiredFormGroup.patchValue(this.competition, { emitEvent: false }); this.updateMinDateBasedOnCompetition(); this.patchCompetitionDateRange(); this.patchRegistrationDateRange(); this.updateCompetitionTypeControl(); this.updateSeatsControls(); } private updateMinDateBasedOnCompetition(): void { if (this.competition.scheduledStartTime) { this.minDate = new Date( new Date(this.competition.scheduledStartTime).setMonth( new Date(this.competition.scheduledStartTime).getMonth() - 1 ) ); } } private patchCompetitionDateRange(): void { if (this.competition.scheduledStartTime && this.competition.scheduledEndTime) { const dateRangeGroup = this.RequiredFormGroup.get('competitionDateRangeGroup'); dateRangeGroup?.patchValue({ start: this.competition.scheduledStartTime, end: this.competition.scheduledEndTime }); dateRangeGroup?.get('start')?.markAsTouched(); dateRangeGroup?.get('end')?.markAsTouched(); } } // ... similar methods for other functionality
212-213: Replace for-in loop with Object.keys iteration.Using for-in with eslint-disable suggests a code smell. Object.keys provides a cleaner approach.
Replace:
// eslint-disable-next-line guard-for-in for (const value in ProviderWorkshopSameValues) { // ... }with:
Object.keys(ProviderWorkshopSameValues).forEach(value => { if (useProviderInfo) { setValue(value); } else { resetValue(value); } });
247-253: Add safety check for undefined controls.The showHintAboutClosingCompetition method assumes that RequiredFormGroup.controls.numberOfSeats exists, but it might not if the form initialization fails.
Add a safety check:
private showHintAboutClosingCompetition(): void { const numberOfSeatsControl = this.RequiredFormGroup.controls.numberOfSeats; if (numberOfSeatsControl) { numberOfSeatsControl.valueChanges.pipe(takeUntil(this.destroy$)).subscribe((availableSeats: number) => { if (availableSeats) { this.isShowHintAboutCompetitionAutoClosing = availableSeats === this.competition?.numberOfOccupiedSeats; } }); } }
255-259: Type safety could be improved in filterTypeOfCompetition method.The method uses type assertions and doesn't fully leverage TypeScript's type system.
Improve type safety:
private filterTypeOfCompetition(stage?: boolean): void { type TypeOfCompetitionKey = keyof typeof TypeOfCompetition; this.filteredTypeOfCompetition = Object.entries(TypeOfCompetition) .filter(([key, value]) => { const numKey = Number(key); return !isNaN(numKey) && (stage || value !== TypeOfCompetition.CompetitionStage); }) .map(([key, value]) => ({ key, value: value as string })); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-competition.component.ts(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.html(1 hunks)src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.ts(1 hunks)
🔇 Additional comments (7)
src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.html (2)
11-30: Validate alignment of label and field for consistent user experience.
Everything looks solid in this title/short title section, but consider verifying that these fields are fully tested in your form submission logic to ensure no accidental omissions or truncations.
33-83: Confirm that numeric input restrictions are enforced.
While you havemaxlength="2"and custom directives (appMinMax), ensure that boundary conditions (e.g. negative, very large numbers) are properly handled at runtime.src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.html (1)
220-223: Potential mismatch betweenareThereBenefitsandbenefitsremains.
This repeats a past comment regarding a mismatch betweenthis.competition.areThereBenefitsandthis.competition.benefits. Verify the intended property usage to avoid logic errors.src/app/shell/personal-cabinet/provider/create-competition/create-competition-description-form/create-competition-description-form.component.ts (1)
220-223: Potential mismatch betweenareThereBenefitsandbenefitsremains.
As previously noted, ensure there's no logical discrepancy by verifying the correct property assignment.src/app/shell/personal-cabinet/provider/create-competition/create-required-form/create-required-form.component.ts (3)
60-62: LGTM! Well-defined getter for form control.The getter provides clean access to the form control without repetitive casting code.
81-96: Add error handling for date operations.Date parsing and form operations should include error handling to prevent runtime errors.
Example implementation:
public ngOnInit(): void { try { this.initForm(); this.PassRequiredFormGroup.emit(this.RequiredFormGroup); this.filterTypeOfCompetition(Boolean(this.parentCompetition)); if (this.competition) { this.activateEditMode(); } if (this.parentCompetition) { this.competitiveEventAccountingTypeIdControl.setValue(TypeOfCompetition.CompetitionStage); this.competitiveEventAccountingTypeIdControl.disable(); } this.initListeners(); } catch (error) { console.error('Failed to initialize form:', error); // Handle error appropriately } }
161-196: Consider enhancing form validation.
- The website, facebook, and instagram fields should validate URLs.
- The registration date range should validate that end date is after start date.
- The competition date range should validate that it's after registration date range.
There are also additional validations that could be added:
- Age validation (minimum age should be less than maximum age)
- Consider adding validation for the coverImageId field if it's required
Implement enhanced validation for age fields:
minimumAge: new FormControl(null, Validators.required), maximumAge: new FormControl(null, Validators.required),to:
minimumAge: new FormControl(null, [ Validators.required, Validators.min(0), Validators.max(100) ]), maximumAge: new FormControl(null, [ Validators.required, Validators.min(0), Validators.max(100) ]),And add a custom validator to ensure minimum age is less than maximum age:
private validateAgeRange(group: FormGroup): ValidationErrors | null { const minAge = group.get('minimumAge')?.value; const maxAge = group.get('maximumAge')?.value; if (minAge !== null && maxAge !== null && minAge >= maxAge) { return { invalidAgeRange: true }; } return null; }
| this.SectionItemsFormArray['_registerControl'](itemFrom); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid direct calls to private Angular APIs.
Calling _registerControl is generally discouraged. Use public FormArray methods (e.g. push) or Angular’s recommended approach to dynamically register controls.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/app/shared/services/competitions/user-competition.service.ts (1)
91-113: 🛠️ Refactor suggestionEnhance type safety in
createFormData.
Currently, you're iterating overObject.keys(competition)with astringtype, which can cause type mismatches. Consider casting or enumerating typed keys.- Object.keys(competition).forEach((key: string) => { + (Object.keys(competition) as Array<keyof Competition>).forEach((key) => {
🧹 Nitpick comments (5)
src/app/shared/store/provider.state.ts (1)
973-984: Avoid using “void” in union types.
Per static analysis, using “void” in anObservable<Competition | void>can be confusing. Replace withundefinedor remove it if not strictly needed:-): Observable<Competition | void> { +): Observable<Competition | undefined> {Also applies to: 1006-1016, 1032-1041
🧰 Tools
🪛 Biome (1.9.4)
[error] 978-978: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/app/shared/services/competitions/user-competition.service.ts (4)
22-28: API version pinned to v1 ingetCompetitionById.
If you intend to switch to v2 under some conditions or a feature flag, ensure that logic is consistently applied here as well.
47-56: Conditional usage of v1 vs. v2 is partially commented out.
Currently, you always callcreateCompetitionV1. Once v2 is ready, consider reintroducing the toggling logic or removing it altogether.
67-85: Same pattern forupdateCompetitionas with creation.
Great consistency. Just be mindful that you skip the v2 path here as well.
87-89: Inconsistent version usage indeleteCompetitionById.
Unlike creation and update, there is no feature flag check for v2. If your application eventually supports file deletion or advanced logic, consider unifying the approach to remain consistent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/assets/i18n/en.jsonis excluded by!src/assets/**and included bysrc/**src/assets/i18n/uk.jsonis excluded by!src/assets/**and included bysrc/**
📒 Files selected for processing (3)
src/app/shared/enum/enumUA/message-bar.ts(1 hunks)src/app/shared/services/competitions/user-competition.service.ts(1 hunks)src/app/shared/store/provider.state.ts(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/shared/enum/enumUA/message-bar.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/app/shared/store/provider.state.ts
[error] 978-978: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 1010-1010: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 1036-1036: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🔇 Additional comments (5)
src/app/shared/store/provider.state.ts (2)
28-28: All new competition imports, model references, property declarations, and injections look good.
They align well with existing naming and state conventions.Also applies to: 31-31, 43-43, 61-61, 77-77, 110-113
336-349: Competition retrieval logic looks consistent with workshop retrieval.
Good job reusing similar patterns and ensuring the state is updated withEMPTY_RESULTif the response is null.src/app/shared/services/competitions/user-competition.service.ts (3)
1-16: Successful integration of store and feature flags.
The injection ofHttpClientandStoreis well-structured, and storingisImagesFeatureat the service level is consistent with how other services handle multi-version features.
30-45: Provider competitions retrieval looks good.
The usage of query params aligns with the existing approach in your application.
58-66: File-based payload withFormDatawell-structured.
createCompetitionV2()aligns with typical multi-part form posting.
| deleteCompetitionByIdSuccess( | ||
| { dispatch }: StateContext<ProviderStateModel>, | ||
| { competition }: providerActions.DeleteCompetitionByIdSuccess | ||
| ): void { | ||
| const messageData = Util.getCompetitionMessage(competition, SnackbarText.deleteCompetition); | ||
| dispatch([ | ||
| new MarkFormDirty(false), | ||
| new ShowMessageBar({ message: messageData.message, type: messageData.type }), | ||
| new providerActions.GetProviderViewCompetitions(competition) | ||
| ]); |
There was a problem hiding this comment.
Mismatch between “competition” and “parameters” in delete competition success action.
Currently, the code dispatches DeleteCompetitionByIdSuccess(parameters) but the reducer interprets that parameter as competition, then uses it to build a message and pass to GetProviderViewCompetitions. This can lead to incorrect or undefined behavior.
A potential fix is to rename the constructor arguments or pass the correct object explicitly, for example:
- deleteCompetitionById(
- { dispatch }: StateContext<ProviderStateModel>,
- { competition, parameters }: providerActions.DeleteCompetitionById
- ): Observable<Competition | void> {
- return this.userCompetitionService.deleteCompetitionById(competition.id).pipe(
- tap(() => dispatch(new providerActions.DeleteCompetitionByIdSuccess(parameters))),
- ...
- );
- }
- deleteCompetitionByIdSuccess(
- { dispatch }: StateContext<ProviderStateModel>,
- { competition }: providerActions.DeleteCompetitionByIdSuccess
- ): void {
- const messageData = Util.getCompetitionMessage(competition, SnackbarText.deleteCompetition);
- dispatch([
- new MarkFormDirty(false),
- new ShowMessageBar({ message: messageData.message, type: messageData.type }),
- new providerActions.GetProviderViewCompetitions(competition)
- ]);
- }
+ deleteCompetitionById(
+ { dispatch }: StateContext<ProviderStateModel>,
+ { competition, parameters }: providerActions.DeleteCompetitionById
+ ): Observable<Competition | undefined> {
+ return this.userCompetitionService.deleteCompetitionById(competition.id).pipe(
+ tap(() => dispatch(new providerActions.DeleteCompetitionByIdSuccess(competition, parameters))),
+ ...
+ );
+ }
+ deleteCompetitionByIdSuccess(
+ { dispatch }: StateContext<ProviderStateModel>,
+ { competition, parameters }: providerActions.DeleteCompetitionByIdSuccess
+ ): void {
+ const messageData = Util.getCompetitionMessage(competition, SnackbarText.deleteCompetition);
+ dispatch([
+ new MarkFormDirty(false),
+ new ShowMessageBar({ message: messageData.message, type: messageData.type }),
+ new providerActions.GetProviderViewCompetitions(parameters)
+ ]);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| deleteCompetitionByIdSuccess( | |
| { dispatch }: StateContext<ProviderStateModel>, | |
| { competition }: providerActions.DeleteCompetitionByIdSuccess | |
| ): void { | |
| const messageData = Util.getCompetitionMessage(competition, SnackbarText.deleteCompetition); | |
| dispatch([ | |
| new MarkFormDirty(false), | |
| new ShowMessageBar({ message: messageData.message, type: messageData.type }), | |
| new providerActions.GetProviderViewCompetitions(competition) | |
| ]); | |
| deleteCompetitionById( | |
| { dispatch }: StateContext<ProviderStateModel>, | |
| { competition, parameters }: providerActions.DeleteCompetitionById | |
| ): Observable<Competition | undefined> { | |
| return this.userCompetitionService.deleteCompetitionById(competition.id).pipe( | |
| tap(() => dispatch(new providerActions.DeleteCompetitionByIdSuccess(competition, parameters))), | |
| // ... other operators or error handling | |
| ); | |
| } | |
| deleteCompetitionByIdSuccess( | |
| { dispatch }: StateContext<ProviderStateModel>, | |
| { competition, parameters }: providerActions.DeleteCompetitionByIdSuccess | |
| ): void { | |
| const messageData = Util.getCompetitionMessage(competition, SnackbarText.deleteCompetition); | |
| dispatch([ | |
| new MarkFormDirty(false), | |
| new ShowMessageBar({ message: messageData.message, type: messageData.type }), | |
| new providerActions.GetProviderViewCompetitions(parameters) | |
| ]); | |
| } |
Implements all components for competition event
Summary by CodeRabbit